diff options
author | Reiner Herrmann <reiner@reiner-h.de> | 2020-07-29 20:22:52 +0200 |
---|---|---|
committer | Reiner Herrmann <reiner@reiner-h.de> | 2020-08-06 17:21:14 +0200 |
commit | 34193604fed04cad2b7b6b0f1a3a0428afd9ed5b (patch) | |
tree | 3be2dd9d6e879eda9639242e6ce9fe434b89b789 | |
parent | firejail: don't interpret output arguments after end-of-options tag (diff) | |
download | firejail-34193604fed04cad2b7b6b0f1a3a0428afd9ed5b.tar.gz firejail-34193604fed04cad2b7b6b0f1a3a0428afd9ed5b.tar.zst firejail-34193604fed04cad2b7b6b0f1a3a0428afd9ed5b.zip |
firejail: don't pass command line through shell when redirecting output
When redirecting output via --output or --output-stderr, firejail was
concatenating all command line arguments into a single string
that was passed to a shell. As the arguments were no longer escaped,
the shell was able to interpret them.
Someone who has control over the command line arguments of the
sandboxed application could use this to run arbitrary other commands.
Instead of passing it through a shell for piping the output to ftee,
the pipeline is now manually created and the processes are executed
directly.
Fixes: CVE-2020-17368
Reported-by: Tim Starling <tstarling@wikimedia.org>
-rw-r--r-- | src/firejail/output.c | 80 |
1 files changed, 54 insertions, 26 deletions
diff --git a/src/firejail/output.c b/src/firejail/output.c index 6e678afd3..0e961bb61 100644 --- a/src/firejail/output.c +++ b/src/firejail/output.c | |||
@@ -77,38 +77,66 @@ void check_output(int argc, char **argv) { | |||
77 | } | 77 | } |
78 | } | 78 | } |
79 | 79 | ||
80 | // build the new command line | 80 | int pipefd[2]; |
81 | int len = 0; | 81 | if (pipe(pipefd) == -1) { |
82 | for (i = 0; i < argc; i++) { | 82 | errExit("pipe"); |
83 | len += strlen(argv[i]) + 1; // + ' ' | ||
84 | } | 83 | } |
85 | len += 100 + strlen(LIBDIR) + strlen(outfile); // tee command | ||
86 | 84 | ||
87 | char *cmd = malloc(len + 1); // + '\0' | 85 | pid_t pid = fork(); |
88 | if (!cmd) | 86 | if (pid == -1) { |
89 | errExit("malloc"); | 87 | errExit("fork"); |
88 | } else if (pid == 0) { | ||
89 | /* child */ | ||
90 | if (dup2(pipefd[0], STDIN_FILENO) == -1) { | ||
91 | errExit("dup2"); | ||
92 | } | ||
93 | close(pipefd[1]); | ||
94 | if (pipefd[0] != STDIN_FILENO) { | ||
95 | close(pipefd[0]); | ||
96 | } | ||
90 | 97 | ||
91 | char *ptr = cmd; | 98 | char *args[3]; |
92 | for (i = 0; i < argc; i++) { | 99 | args[0] = LIBDIR "/firejail/ftee"; |
93 | if (strncmp(argv[i], "--output=", 9) == 0) | 100 | args[1] = outfile; |
94 | continue; | 101 | args[2] = NULL; |
95 | if (strncmp(argv[i], "--output-stderr=", 16) == 0) | 102 | execv(args[0], args); |
96 | continue; | 103 | perror("execvp"); |
97 | ptr += sprintf(ptr, "%s ", argv[i]); | 104 | exit(1); |
98 | } | 105 | } |
99 | 106 | ||
100 | if (enable_stderr) | 107 | /* parent */ |
101 | sprintf(ptr, "2>&1 | %s/firejail/ftee %s", LIBDIR, outfile); | 108 | if (dup2(pipefd[1], STDOUT_FILENO) == -1) { |
102 | else | 109 | errExit("dup2"); |
103 | sprintf(ptr, " | %s/firejail/ftee %s", LIBDIR, outfile); | 110 | } |
111 | if (enable_stderr && dup2(STDOUT_FILENO, STDERR_FILENO) == -1) { | ||
112 | errExit("dup2"); | ||
113 | } | ||
114 | close(pipefd[0]); | ||
115 | if (pipefd[1] != STDOUT_FILENO) { | ||
116 | close(pipefd[1]); | ||
117 | } | ||
104 | 118 | ||
105 | // run command | 119 | char **args = calloc(argc + 1, sizeof(char *)); |
106 | char *a[4]; | 120 | if (!args) { |
107 | a[0] = "/bin/bash"; | 121 | errExit("calloc"); |
108 | a[1] = "-c"; | 122 | } |
109 | a[2] = cmd; | 123 | bool found_separator = false; |
110 | a[3] = NULL; | 124 | /* copy argv into args, but drop --output(-stderr) arguments */ |
111 | execvp(a[0], a); | 125 | for (int i = 0, j = 0; i < argc; i++) { |
126 | if (!found_separator && i > 0) { | ||
127 | if (strncmp(argv[i], "--output=", 9) == 0) { | ||
128 | continue; | ||
129 | } | ||
130 | if (strncmp(argv[i], "--output-stderr=", 16) == 0) { | ||
131 | continue; | ||
132 | } | ||
133 | if (strncmp(argv[i], "--", 2) != 0 || strcmp(argv[i], "--") == 0) { | ||
134 | found_separator = true; | ||
135 | } | ||
136 | } | ||
137 | args[j++] = argv[i]; | ||
138 | } | ||
139 | execvp(args[0], args); | ||
112 | 140 | ||
113 | perror("execvp"); | 141 | perror("execvp"); |
114 | exit(1); | 142 | exit(1); |