diff options
author | Jeff Squyres <jsquyres@cisco.com> | 2021-05-28 13:44:42 -0700 |
---|---|---|
committer | Jeff Squyres <jsquyres@cisco.com> | 2021-06-02 16:08:58 -0700 |
commit | afb7d07f667471f4ba5505736915b4307413ef2d (patch) | |
tree | 880451aaa3022aab53d83e7ff504626992197878 /src | |
parent | reorganizing youtube-viewers (#4128) (diff) | |
download | firejail-afb7d07f667471f4ba5505736915b4307413ef2d.tar.gz firejail-afb7d07f667471f4ba5505736915b4307413ef2d.tar.zst firejail-afb7d07f667471f4ba5505736915b4307413ef2d.zip |
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>
Diffstat (limited to 'src')
-rw-r--r-- | src/firejail/cmdline.c | 32 | ||||
-rw-r--r-- | src/firejail/firejail.h | 4 | ||||
-rw-r--r-- | src/firejail/join.c | 2 | ||||
-rw-r--r-- | src/firejail/main.c | 5 | ||||
-rw-r--r-- | src/firejail/no_sandbox.c | 2 |
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 @@ | |||
26 | #include <assert.h> | 26 | #include <assert.h> |
27 | #include <errno.h> | 27 | #include <errno.h> |
28 | 28 | ||
29 | static int cmdline_length(int argc, char **argv, int index) { | 29 | static int cmdline_length(int argc, char **argv, int index, bool want_extra_quotes) { |
30 | assert(index != -1); | 30 | assert(index != -1); |
31 | 31 | ||
32 | unsigned i,j; | 32 | unsigned i,j; |
@@ -46,10 +46,11 @@ static int cmdline_length(int argc, char **argv, int index) { | |||
46 | len += 3; | 46 | len += 3; |
47 | in_quotes = false; | 47 | in_quotes = false; |
48 | } else { | 48 | } else { |
49 | if (!in_quotes) | 49 | if (!in_quotes && want_extra_quotes) |
50 | len++; | 50 | len++; |
51 | len++; | 51 | len++; |
52 | in_quotes = true; | 52 | if (want_extra_quotes) |
53 | in_quotes = true; | ||
53 | } | 54 | } |
54 | } | 55 | } |
55 | if (in_quotes) { | 56 | if (in_quotes) { |
@@ -64,7 +65,7 @@ static int cmdline_length(int argc, char **argv, int index) { | |||
64 | return len; | 65 | return len; |
65 | } | 66 | } |
66 | 67 | ||
67 | static void quote_cmdline(char *command_line, char *window_title, int len, int argc, char **argv, int index) { | 68 | static void quote_cmdline(char *command_line, char *window_title, int len, int argc, char **argv, int index, bool want_extra_quotes) { |
68 | assert(index != -1); | 69 | assert(index != -1); |
69 | 70 | ||
70 | unsigned i,j; | 71 | unsigned i,j; |
@@ -103,14 +104,15 @@ static void quote_cmdline(char *command_line, char *window_title, int len, int a | |||
103 | // anything other | 104 | // anything other |
104 | else | 105 | else |
105 | { | 106 | { |
106 | if (!in_quotes) { | 107 | if (!in_quotes && want_extra_quotes) { |
107 | // open quotes | 108 | // open quotes |
108 | ptr1[0] = '\''; | 109 | ptr1[0] = '\''; |
109 | ptr1++; | 110 | ptr1++; |
110 | } | 111 | } |
111 | ptr1[0] = argv[i + index][j]; | 112 | ptr1[0] = argv[i + index][j]; |
112 | ptr1++; | 113 | ptr1++; |
113 | in_quotes = true; | 114 | if (want_extra_quotes) |
115 | in_quotes = true; | ||
114 | } | 116 | } |
115 | } | 117 | } |
116 | // close quotes | 118 | // close quotes |
@@ -134,12 +136,12 @@ static void quote_cmdline(char *command_line, char *window_title, int len, int a | |||
134 | assert((unsigned) len == strlen(command_line)); | 136 | assert((unsigned) len == strlen(command_line)); |
135 | } | 137 | } |
136 | 138 | ||
137 | void build_cmdline(char **command_line, char **window_title, int argc, char **argv, int index) { | 139 | void build_cmdline(char **command_line, char **window_title, int argc, char **argv, int index, bool want_extra_quotes) { |
138 | // index == -1 could happen if we have --shell=none and no program was specified | 140 | // index == -1 could happen if we have --shell=none and no program was specified |
139 | // the program should exit with an error before entering this function | 141 | // the program should exit with an error before entering this function |
140 | assert(index != -1); | 142 | assert(index != -1); |
141 | 143 | ||
142 | int len = cmdline_length(argc, argv, index); | 144 | int len = cmdline_length(argc, argv, index, want_extra_quotes); |
143 | if (len > ARG_MAX) { | 145 | if (len > ARG_MAX) { |
144 | errno = E2BIG; | 146 | errno = E2BIG; |
145 | errExit("cmdline_length"); | 147 | errExit("cmdline_length"); |
@@ -152,7 +154,7 @@ void build_cmdline(char **command_line, char **window_title, int argc, char **ar | |||
152 | if (!*window_title) | 154 | if (!*window_title) |
153 | errExit("malloc"); | 155 | errExit("malloc"); |
154 | 156 | ||
155 | quote_cmdline(*command_line, *window_title, len, argc, argv, index); | 157 | quote_cmdline(*command_line, *window_title, len, argc, argv, index, want_extra_quotes); |
156 | 158 | ||
157 | if (arg_debug) | 159 | if (arg_debug) |
158 | printf("Building quoted command line: %s\n", *command_line); | 160 | 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 | |||
161 | assert(*window_title); | 163 | assert(*window_title); |
162 | } | 164 | } |
163 | 165 | ||
164 | void build_appimage_cmdline(char **command_line, char **window_title, int argc, char **argv, int index) { | 166 | void build_appimage_cmdline(char **command_line, char **window_title, int argc, char **argv, int index, bool want_extra_quotes) { |
165 | // index == -1 could happen if we have --shell=none and no program was specified | 167 | // index == -1 could happen if we have --shell=none and no program was specified |
166 | // the program should exit with an error before entering this function | 168 | // the program should exit with an error before entering this function |
167 | assert(index != -1); | 169 | assert(index != -1); |
168 | 170 | ||
169 | char *apprun_path = RUN_FIREJAIL_APPIMAGE_DIR "/AppRun"; | 171 | char *apprun_path = RUN_FIREJAIL_APPIMAGE_DIR "/AppRun"; |
170 | 172 | ||
171 | int len1 = cmdline_length(argc, argv, index); // length of argv w/o changes | 173 | int len1 = cmdline_length(argc, argv, index, want_extra_quotes); // length of argv w/o changes |
172 | int len2 = cmdline_length(1, &argv[index], 0); // apptest.AppImage | 174 | int len2 = cmdline_length(1, &argv[index], 0, want_extra_quotes); // apptest.AppImage |
173 | int len3 = cmdline_length(1, &apprun_path, 0); // /run/firejail/appimage/AppRun | 175 | int len3 = cmdline_length(1, &apprun_path, 0, want_extra_quotes); // /run/firejail/appimage/AppRun |
174 | int len4 = (len1 - len2 + len3) + 1; // apptest.AppImage is replaced by /path/to/AppRun | 176 | int len4 = (len1 - len2 + len3) + 1; // apptest.AppImage is replaced by /path/to/AppRun |
175 | 177 | ||
176 | if (len4 > ARG_MAX) { | 178 | if (len4 > ARG_MAX) { |
177 | errno = E2BIG; | 179 | errno = E2BIG; |
@@ -187,7 +189,7 @@ void build_appimage_cmdline(char **command_line, char **window_title, int argc, | |||
187 | errExit("malloc"); | 189 | errExit("malloc"); |
188 | 190 | ||
189 | // run default quote_cmdline | 191 | // run default quote_cmdline |
190 | quote_cmdline(command_line_tmp, *window_title, len1, argc, argv, index); | 192 | quote_cmdline(command_line_tmp, *window_title, len1, argc, argv, index, want_extra_quotes); |
191 | 193 | ||
192 | assert(command_line_tmp); | 194 | assert(command_line_tmp); |
193 | assert(*window_title); | 195 | 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); | |||
803 | long unsigned int appimage2_size(int fd); | 803 | long unsigned int appimage2_size(int fd); |
804 | 804 | ||
805 | // cmdline.c | 805 | // cmdline.c |
806 | void build_cmdline(char **command_line, char **window_title, int argc, char **argv, int index); | 806 | void build_cmdline(char **command_line, char **window_title, int argc, char **argv, int index, bool want_extra_quotes); |
807 | void build_appimage_cmdline(char **command_line, char **window_title, int argc, char **argv, int index); | 807 | void build_appimage_cmdline(char **command_line, char **window_title, int argc, char **argv, int index, bool want_extra_quotes); |
808 | 808 | ||
809 | // sbox.c | 809 | // sbox.c |
810 | // programs | 810 | // 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) { | |||
147 | } | 147 | } |
148 | 148 | ||
149 | // build command | 149 | // build command |
150 | build_cmdline(&cfg.command_line, &cfg.window_title, argc, argv, index); | 150 | build_cmdline(&cfg.command_line, &cfg.window_title, argc, argv, index, true); |
151 | } | 151 | } |
152 | 152 | ||
153 | static void extract_nogroups(pid_t pid) { | 153 | 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) { | |||
2810 | if (arg_debug) | 2810 | if (arg_debug) |
2811 | printf("Configuring appimage environment\n"); | 2811 | printf("Configuring appimage environment\n"); |
2812 | appimage_set(cfg.command_name); | 2812 | appimage_set(cfg.command_name); |
2813 | build_appimage_cmdline(&cfg.command_line, &cfg.window_title, argc, argv, prog_index); | 2813 | build_appimage_cmdline(&cfg.command_line, &cfg.window_title, argc, argv, prog_index, true); |
2814 | } | 2814 | } |
2815 | else { | 2815 | else { |
2816 | build_cmdline(&cfg.command_line, &cfg.window_title, argc, argv, prog_index); | 2816 | // Only add extra quotes if we were not launched by sshd. |
2817 | build_cmdline(&cfg.command_line, &cfg.window_title, argc, argv, prog_index, !parent_sshd); | ||
2817 | } | 2818 | } |
2818 | /* else { | 2819 | /* else { |
2819 | fprintf(stderr, "Error: command must be specified when --shell=none used.\n"); | 2820 | 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) { | |||
204 | // force --shell=none in order to not break firecfg symbolic links | 204 | // force --shell=none in order to not break firecfg symbolic links |
205 | arg_shell_none = 1; | 205 | arg_shell_none = 1; |
206 | 206 | ||
207 | build_cmdline(&cfg.command_line, &cfg.window_title, argc, argv, prog_index); | 207 | build_cmdline(&cfg.command_line, &cfg.window_title, argc, argv, prog_index, true); |
208 | } | 208 | } |
209 | 209 | ||
210 | fwarning("an existing sandbox was detected. " | 210 | fwarning("an existing sandbox was detected. " |