From 68b9c1382593fabc0e58862a45b4d010e8bb2256 Mon Sep 17 00:00:00 2001 From: Aleksey Manevich Date: Tue, 12 Jul 2016 05:22:47 +0300 Subject: Another command line quoting fix 1. Arguments passed to shell should always be quoted by single quotes. 2. Arguments passed directly to program (--shell=none) should never be quoted. --- src/firejail/main.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/firejail/main.c b/src/firejail/main.c index 232a57499..366b41f88 100644 --- a/src/firejail/main.c +++ b/src/firejail/main.c @@ -2022,11 +2022,7 @@ int main(int argc, char **argv) { char *ptr1 = cfg.command_line; char *ptr2 = cfg.window_title; for (i = 0; i < argcnt; i++) { - // detect bash commands - if (strstr(argv[i + prog_index], "&&") || strstr(argv[i + prog_index], "||")) { - sprintf(ptr1, "%s ", argv[i + prog_index]); - } - else if (arg_command){ + if (arg_shell_none){ sprintf(ptr1, "%s ", argv[i + prog_index]); } else { -- cgit v1.2.3-70-g09d2 From 0a79f8ebe8201f52495b26f6b28b6eb7c553d7ff Mon Sep 17 00:00:00 2001 From: Aleksey Manevich Date: Tue, 12 Jul 2016 05:28:41 +0300 Subject: Remove redundant code This code also causes obscure errors in some rare cases --- src/firejail/run_symlink.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/firejail/run_symlink.c b/src/firejail/run_symlink.c index 26c6c5133..020e70b80 100644 --- a/src/firejail/run_symlink.c +++ b/src/firejail/run_symlink.c @@ -103,16 +103,7 @@ void run_symlink(int argc, char **argv) { a[1] = program; int i; for (i = 0; i < (argc - 1); i++) { - // look for & character - if (strchr(argv[i + 1], '&')) { - char *str = malloc(strlen(argv[i + 1])); - if (str == NULL) - errExit("malloc"); - sprintf(str, "\"%s\"", argv[i + 1]); - a[i + 2] = str; - } - else - a[i + 2] = argv[i + 1]; + a[i + 2] = argv[i + 1]; } a[i + 2] = NULL; execvp(a[0], a); -- cgit v1.2.3-70-g09d2 From 834ca520e8a54291c91e46d9a3e10dce9b806e57 Mon Sep 17 00:00:00 2001 From: Aleksey Manevich Date: Tue, 12 Jul 2016 07:00:46 +0300 Subject: Small fix args prepared here are only for shell --- src/firejail/main.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/firejail/main.c b/src/firejail/main.c index 366b41f88..91f39ed71 100644 --- a/src/firejail/main.c +++ b/src/firejail/main.c @@ -2022,12 +2022,7 @@ int main(int argc, char **argv) { char *ptr1 = cfg.command_line; char *ptr2 = cfg.window_title; for (i = 0; i < argcnt; i++) { - if (arg_shell_none){ - sprintf(ptr1, "%s ", argv[i + prog_index]); - } - else { - sprintf(ptr1, "\'%s\' ", argv[i + prog_index]); - } + sprintf(ptr1, "\'%s\' ", argv[i + prog_index]); sprintf(ptr2, "%s ", argv[i + prog_index]); ptr1 += strlen(ptr1); -- cgit v1.2.3-70-g09d2 From 7669b9410df5639365967c57326a3fc3bb192810 Mon Sep 17 00:00:00 2001 From: Aleksey Manevich Date: Sat, 16 Jul 2016 00:34:07 +0300 Subject: Fix problem with single quotes in args Single quotes can't be represented in single quoted text, so quote them separately by double quotes. --- src/firejail/main.c | 45 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/src/firejail/main.c b/src/firejail/main.c index 91f39ed71..28351a2df 100644 --- a/src/firejail/main.c +++ b/src/firejail/main.c @@ -2008,8 +2008,26 @@ int main(int argc, char **argv) { int i; int len = 0; int argcnt = argc - prog_index; - for (i = 0; i < argcnt; i++) - len += strlen(argv[i + prog_index]) + 3; // + ' ' + 2 '"' + int j; + char *arg, *arg_ptr, *token; + + for (i = 0; i < argcnt; i++) { + arg = strdup(argv[i + prog_index]); + arg_ptr = arg; + for (token = strsep(&arg_ptr, "\'"); token != NULL; token = strsep(&arg_ptr, "\'")) { + if (token[0] == '\0') { + len += 3; + } else { + len += strlen(token) + 5; + } + } + free(arg); + len -= 2; // + ' ' - 3 char overrun + } + len += 3; // for overrun + + if (arg_debug) + printf("Predicted command length %d\n", len); // build the string cfg.command_line = malloc(len + 1); // + '\0' @@ -2022,12 +2040,31 @@ int main(int argc, char **argv) { char *ptr1 = cfg.command_line; char *ptr2 = cfg.window_title; for (i = 0; i < argcnt; i++) { - sprintf(ptr1, "\'%s\' ", argv[i + prog_index]); - sprintf(ptr2, "%s ", argv[i + prog_index]); + // enclose args by single quotes, + // and since single quote can't be represented in single quoted text + // each occurence of it in arg should be enclosed by double quotes + arg = strdup(argv[i + prog_index]); + arg_ptr = arg; + for (token = strsep(&arg_ptr, "\'"); token != NULL; token = strsep(&arg_ptr, "\'")) { + if (token[0] == '\0') { + sprintf(ptr1, "\"\'\""); + } else { + sprintf(ptr1, "\'%s\'\"\'\"", token); + } + ptr1 += strlen(ptr1); + } + free(arg); + ptr1 -= 3; + sprintf(ptr1, " "); ptr1 += strlen(ptr1); + + sprintf(ptr2, "%s ", argv[i + prog_index]); ptr2 += strlen(ptr2); } + ptr1[0]='\0'; // just to be sure + if (arg_debug) + printf("Actual command length %zd\n", strlen(cfg.command_line)); } assert(cfg.command_name); -- cgit v1.2.3-70-g09d2 From 1f4e6d3888130eff5485cbdd6ff4e84aa6e69f96 Mon Sep 17 00:00:00 2001 From: Aleksey Manevich Date: Tue, 19 Jul 2016 15:45:55 +0300 Subject: Fix problem with single quotes in args Single quotes can't be represented in single quoted text, so quote them separately by double quotes. Better version. --- src/firejail/main.c | 96 ++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 69 insertions(+), 27 deletions(-) diff --git a/src/firejail/main.c b/src/firejail/main.c index 28351a2df..4f1c81e2b 100644 --- a/src/firejail/main.c +++ b/src/firejail/main.c @@ -2009,25 +2009,34 @@ int main(int argc, char **argv) { int len = 0; int argcnt = argc - prog_index; int j; - char *arg, *arg_ptr, *token; + bool in_quotes = false; for (i = 0; i < argcnt; i++) { - arg = strdup(argv[i + prog_index]); - arg_ptr = arg; - for (token = strsep(&arg_ptr, "\'"); token != NULL; token = strsep(&arg_ptr, "\'")) { - if (token[0] == '\0') { - len += 3; + in_quotes = false; + for (j = 0; j < strlen(argv[i + prog_index]); j++) { + if (argv[i + prog_index][j] == '\'') { + if (in_quotes) + len++; + if (j > 0 && argv[i + prog_index][j-1] == '\'') + len++; + else + len += 3; + in_quotes = false; } else { - len += strlen(token) + 5; + if (!in_quotes) + len++; + len++; + in_quotes = true; } } - free(arg); - len -= 2; // + ' ' - 3 char overrun + if (in_quotes) { + len++; + } + if (strlen(argv[i + prog_index]) == 0) { + len += 2; + } + len++; } - len += 3; // for overrun - - if (arg_debug) - printf("Predicted command length %d\n", len); // build the string cfg.command_line = malloc(len + 1); // + '\0' @@ -2040,31 +2049,64 @@ int main(int argc, char **argv) { char *ptr1 = cfg.command_line; char *ptr2 = cfg.window_title; for (i = 0; i < argcnt; i++) { + // enclose args by single quotes, // and since single quote can't be represented in single quoted text - // each occurence of it in arg should be enclosed by double quotes - arg = strdup(argv[i + prog_index]); - arg_ptr = arg; - for (token = strsep(&arg_ptr, "\'"); token != NULL; token = strsep(&arg_ptr, "\'")) { - if (token[0] == '\0') { - sprintf(ptr1, "\"\'\""); - } else { - sprintf(ptr1, "\'%s\'\"\'\"", token); + // each occurence of it should be enclosed by double quotes + in_quotes = false; + for (j = 0; j < strlen(argv[i + prog_index]); j++) { + // single quote + if (argv[i + prog_index][j] == '\'') { + if (in_quotes) { + // close quotes + ptr1[0] = '\''; + ptr1++; + } + // previous char was single quote too + if (j > 0 && argv[i + prog_index][j-1] == '\'') { + ptr1--; + sprintf(ptr1, "\'\""); + } + // this first in series + else + { + sprintf(ptr1, "\"\'\""); + } + ptr1 += strlen(ptr1); + in_quotes = false; } + // anything other + else + { + if (!in_quotes) { + // open quotes + ptr1[0] = '\''; + ptr1++; + } + ptr1[0] = argv[i + prog_index][j]; + ptr1++; + in_quotes = true; + } + } + // close quotes + if (in_quotes) { + ptr1[0] = '\''; + ptr1++; + } + // handle empty argument case + if (strlen(argv[i + prog_index]) == 0) { + sprintf(ptr1, "\'\'"); ptr1 += strlen(ptr1); } - free(arg); - ptr1 -= 3; - + // add space sprintf(ptr1, " "); ptr1 += strlen(ptr1); sprintf(ptr2, "%s ", argv[i + prog_index]); ptr2 += strlen(ptr2); } - ptr1[0]='\0'; // just to be sure - if (arg_debug) - printf("Actual command length %zd\n", strlen(cfg.command_line)); + + assert(len == strlen(cfg.command_line)); } assert(cfg.command_name); -- cgit v1.2.3-70-g09d2