cmdline.c: optionally quote the resulting command line

If we were launched by sshd, do not add extra quotes to the command
line.  This is because if firejail is a login shell, sshd will launch
firejail thusly:

* argv[0]: /path/to/firejail
* argv[1]: -c
* argv[2]: user's command to execute

For example, if the user executed "ssh othernode echo hello world",
argv[2] will be "echo hello world".  Firejail will then add *extra*
quotes to it, resulting in argv[2] becoming "'echo hello world' "
(without the "", of course).  The user's shell (e.g., bash) will see
the extra single quotes and will not split the token into multiple
tokens.  The shell will be unable to find an executable or intrinsic
named "echo hello world ", so it will fail.

This commit changes the above behavior if firejail is launched by
sshd.  In that case, firejail will *not* add the extra single quotes
around argv[2].  Specifically: all the tokens still end up in argv[2],
but there's no *extra* quotes around argv[2], so the shell will split
argv[2] into multiple tokens (if necessary).  In the above example,
argv[2] will be "echo hello world" (without the ""), which will be
split.  The shell will then look for an intrinsic or executable named
"echo", which will succeed, and "hello world" will ultimately be
emitted.

Signed-off-by: Jeff Squyres <jsquyres@cisco.com>

5 files changed, 24 insertions, 21 deletions

diff --git a/src/firejail/cmdline.c b/src/firejail/cmdline.c
index f902c4e1c..2fa68a55d 100644
--- a/src/firejail/cmdline.c
+++ b/src/firejail/cmdline.c
@@ -26,7 +26,7 @@
 #include <assert.h>
 #include <errno.h>
 
-static int cmdline_length(int argc, char **argv, int index) {
+static int cmdline_length(int argc, char **argv, int index, bool want_extra_quotes) {
         assert(index != -1);
 
         unsigned i,j;
@@ -46,10 +46,11 @@ static int cmdline_length(int argc, char **argv, int index) {
                                         len += 3;
                                 in_quotes = false;
                         } else {
-                                if (!in_quotes)
+                                if (!in_quotes && want_extra_quotes)
                                         len++;
                                 len++;
-                                in_quotes = true;
+                                if (want_extra_quotes)
+                                        in_quotes = true;
                         }
                 }
                 if (in_quotes) {
@@ -64,7 +65,7 @@ static int cmdline_length(int argc, char **argv, int index) {
         return len;
 }
 
-static void quote_cmdline(char *command_line, char *window_title, int len, int argc, char **argv, int index) {
+static void quote_cmdline(char *command_line, char *window_title, int len, int argc, char **argv, int index, bool want_extra_quotes) {
         assert(index != -1);
 
         unsigned i,j;
@@ -103,14 +104,15 @@ static void quote_cmdline(char *command_line, char *window_title, int len, int a
                         // anything other
                         else
                         {
-                                if (!in_quotes) {
+                                if (!in_quotes && want_extra_quotes) {
                                         // open quotes
                                         ptr1[0] = '\'';
                                         ptr1++;
                                 }
                                 ptr1[0] = argv[i + index][j];
                                 ptr1++;
-                                in_quotes = true;
+                                if (want_extra_quotes)
+                                        in_quotes = true;
                         }
                 }
                 // close quotes
@@ -134,12 +136,12 @@ static void quote_cmdline(char *command_line, char *window_title, int len, int a
         assert((unsigned) len == strlen(command_line));
 }
 
-void build_cmdline(char **command_line, char **window_title, int argc, char **argv, int index) {
+void build_cmdline(char **command_line, char **window_title, int argc, char **argv, int index, bool want_extra_quotes) {
         // index == -1 could happen if we have --shell=none and no program was specified
         // the program should exit with an error before entering this function
         assert(index != -1);
 
-        int len = cmdline_length(argc, argv, index);
+        int len = cmdline_length(argc, argv, index, want_extra_quotes);
         if (len > ARG_MAX) {
                 errno = E2BIG;
                 errExit("cmdline_length");
@@ -152,7 +154,7 @@ void build_cmdline(char **command_line, char **window_title, int argc, char **ar
         if (!*window_title)
                         errExit("malloc");
 
-        quote_cmdline(*command_line, *window_title, len, argc, argv, index);
+        quote_cmdline(*command_line, *window_title, len, argc, argv, index, want_extra_quotes);
 
         if (arg_debug)
                 printf("Building quoted command line: %s\n", *command_line);
@@ -161,17 +163,17 @@ void build_cmdline(char **command_line, char **window_title, int argc, char **ar
         assert(*window_title);
 }
 
-void build_appimage_cmdline(char **command_line, char **window_title, int argc, char **argv, int index) {
+void build_appimage_cmdline(char **command_line, char **window_title, int argc, char **argv, int index, bool want_extra_quotes) {
         // index == -1 could happen if we have --shell=none and no program was specified
         // the program should exit with an error before entering this function
         assert(index != -1);
 
         char *apprun_path = RUN_FIREJAIL_APPIMAGE_DIR "/AppRun";
 
-        int len1 = cmdline_length(argc, argv, index);  // length of argv w/o changes
-        int len2 = cmdline_length(1, &argv[index], 0); // apptest.AppImage
-        int len3 = cmdline_length(1, &apprun_path, 0); // /run/firejail/appimage/AppRun
-        int len4 = (len1 - len2 + len3) + 1;           // apptest.AppImage is replaced by /path/to/AppRun
+        int len1 = cmdline_length(argc, argv, index, want_extra_quotes);  // length of argv w/o changes
+        int len2 = cmdline_length(1, &argv[index], 0, want_extra_quotes); // apptest.AppImage
+        int len3 = cmdline_length(1, &apprun_path, 0, want_extra_quotes); // /run/firejail/appimage/AppRun
+        int len4 = (len1 - len2 + len3) + 1;                              // apptest.AppImage is replaced by /path/to/AppRun
 
         if (len4 > ARG_MAX) {
                 errno = E2BIG;
@@ -187,7 +189,7 @@ void build_appimage_cmdline(char **command_line, char **window_title, int argc,
                         errExit("malloc");
 
         // run default quote_cmdline
-        quote_cmdline(command_line_tmp, *window_title, len1, argc, argv, index);
+        quote_cmdline(command_line_tmp, *window_title, len1, argc, argv, index, want_extra_quotes);
 
         assert(command_line_tmp);
         assert(*window_title);
diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h
index 1da70fd54..ef015fc70 100644
--- a/src/firejail/firejail.h
+++ b/src/firejail/firejail.h
@@ -803,8 +803,8 @@ void appimage_clear(void);
 long unsigned int appimage2_size(int fd);
 
 // cmdline.c
-void build_cmdline(char **command_line, char **window_title, int argc, char **argv, int index);
-void build_appimage_cmdline(char **command_line, char **window_title, int argc, char **argv, int index);
+void build_cmdline(char **command_line, char **window_title, int argc, char **argv, int index, bool want_extra_quotes);
+void build_appimage_cmdline(char **command_line, char **window_title, int argc, char **argv, int index, bool want_extra_quotes);
 
 // sbox.c
 // programs
diff --git a/src/firejail/join.c b/src/firejail/join.c
index bab4b830f..394bbb528 100644
--- a/src/firejail/join.c
+++ b/src/firejail/join.c
@@ -147,7 +147,7 @@ static void extract_command(int argc, char **argv, int index) {
         }
 
         // build command
-        build_cmdline(&cfg.command_line, &cfg.window_title, argc, argv, index);
+        build_cmdline(&cfg.command_line, &cfg.window_title, argc, argv, index, true);
 }
 
 static void extract_nogroups(pid_t pid) {
diff --git a/src/firejail/main.c b/src/firejail/main.c
index 31694558d..72c60a1bc 100644
--- a/src/firejail/main.c
+++ b/src/firejail/main.c
@@ -2810,10 +2810,11 @@ int main(int argc, char **argv, char **envp) {
                 if (arg_debug)
                         printf("Configuring appimage environment\n");
                 appimage_set(cfg.command_name);
-                build_appimage_cmdline(&cfg.command_line, &cfg.window_title, argc, argv, prog_index);
+                build_appimage_cmdline(&cfg.command_line, &cfg.window_title, argc, argv, prog_index, true);
         }
         else {
-                build_cmdline(&cfg.command_line, &cfg.window_title, argc, argv, prog_index);
+                // Only add extra quotes if we were not launched by sshd.
+                build_cmdline(&cfg.command_line, &cfg.window_title, argc, argv, prog_index, !parent_sshd);
         }
 /*      else {
                 fprintf(stderr, "Error: command must be specified when --shell=none used.\n");
diff --git a/src/firejail/no_sandbox.c b/src/firejail/no_sandbox.c
index 0e153c47b..665bef73d 100644
--- a/src/firejail/no_sandbox.c
+++ b/src/firejail/no_sandbox.c
@@ -204,7 +204,7 @@ void run_no_sandbox(int argc, char **argv) {
                 // force --shell=none in order to not break firecfg symbolic links
                 arg_shell_none = 1;
 
-                build_cmdline(&cfg.command_line, &cfg.window_title, argc, argv, prog_index);
+                build_cmdline(&cfg.command_line, &cfg.window_title, argc, argv, prog_index, true);
         }
 
         fwarning("an existing sandbox was detected. "