diff --git a/CHANGES b/CHANGES index 20a25c551e..733c4ed9dc 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,8 @@ +2.6-192 | 2019-03-25 17:49:18 -0700 + + * Deprecate str_shell_escape, add safe_shell_quote replacement (Jon Siwek, Corelight) + 2.6-191 | 2019-03-25 16:43:10 -0700 * Add support for SMB filenames to the intel framework (Stephen Hosom) diff --git a/NEWS b/NEWS index d673e3385e..09763cdd0e 100644 --- a/NEWS +++ b/NEWS @@ -106,6 +106,10 @@ Removed Functionality Deprecated Functionality ------------------------ +- The ``str_shell_escape` function is now deprecated, use ``safe_shell_quote`` + instead. The later will automatically return a value that is enclosed + in double-quotes. + Bro 2.6 ======= diff --git a/VERSION b/VERSION index 73ef942bdc..e7ef7ca154 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.6-191 +2.6-192 diff --git a/doc b/doc index 406d0c8574..e404fc80c5 160000 --- a/doc +++ b/doc @@ -1 +1 @@ -Subproject commit 406d0c857491927d7fbee7aef954b8a40f23978d +Subproject commit e404fc80c5c4ecfd0c4441b6b83826761bd985e9 diff --git a/scripts/base/utils/active-http.bro b/scripts/base/utils/active-http.bro index a6b0f8111c..8243a7a9a9 100644 --- a/scripts/base/utils/active-http.bro +++ b/scripts/base/utils/active-http.bro @@ -57,10 +57,10 @@ export { function request2curl(r: Request, bodyfile: string, headersfile: string): string { - local cmd = fmt("curl -s -g -o \"%s\" -D \"%s\" -X \"%s\"", - str_shell_escape(bodyfile), - str_shell_escape(headersfile), - str_shell_escape(r$method)); + local cmd = fmt("curl -s -g -o %s -D %s -X %s", + safe_shell_quote(bodyfile), + safe_shell_quote(headersfile), + safe_shell_quote(r$method)); cmd = fmt("%s -m %.0f", cmd, r$max_time); @@ -70,9 +70,9 @@ function request2curl(r: Request, bodyfile: string, headersfile: string): string if ( r?$addl_curl_args ) cmd = fmt("%s %s", cmd, r$addl_curl_args); - cmd = fmt("%s \"%s\"", cmd, str_shell_escape(r$url)); + cmd = fmt("%s %s", cmd, safe_shell_quote(r$url)); # Make sure file will exist even if curl did not write one. - cmd = fmt("%s && touch %s", cmd, str_shell_escape(bodyfile)); + cmd = fmt("%s && touch %s", cmd, safe_shell_quote(bodyfile)); return cmd; } diff --git a/scripts/base/utils/dir.bro b/scripts/base/utils/dir.bro index c3598d039d..eb5597a7b7 100644 --- a/scripts/base/utils/dir.bro +++ b/scripts/base/utils/dir.bro @@ -28,7 +28,7 @@ event Dir::monitor_ev(dir: string, last_files: set[string], callback: function(fname: string), poll_interval: interval) { - when ( local result = Exec::run([$cmd=fmt("ls -1 \"%s/\"", str_shell_escape(dir))]) ) + when ( local result = Exec::run([$cmd=fmt("ls -1 %s/", safe_shell_quote(dir))]) ) { if ( result$exit_code != 0 ) { diff --git a/scripts/base/utils/exec.bro b/scripts/base/utils/exec.bro index 37668c0bc6..91053a1223 100644 --- a/scripts/base/utils/exec.bro +++ b/scripts/base/utils/exec.bro @@ -8,7 +8,7 @@ export { type Command: record { ## The command line to execute. Use care to avoid injection ## attacks (i.e., if the command uses untrusted/variable data, - ## sanitize it with :bro:see:`str_shell_escape`). + ## sanitize it with :bro:see:`safe_shell_quote`). cmd: string; ## Provide standard input to the program as a string. stdin: string &default=""; @@ -122,7 +122,7 @@ event Input::end_of_data(orig_name: string, source:string) delete pending_files[name][track_file]; if ( |pending_files[name]| == 0 ) delete pending_commands[name]; - system(fmt("rm \"%s\"", str_shell_escape(track_file))); + system(fmt("rm %s", safe_shell_quote(track_file))); } } @@ -191,5 +191,5 @@ event bro_done() # We are punting here and just deleting any unprocessed files. for ( uid in pending_files ) for ( fname in pending_files[uid] ) - system(fmt("rm \"%s\"", str_shell_escape(fname))); + system(fmt("rm %s", safe_shell_quote(fname))); } diff --git a/src/bro.bif b/src/bro.bif index 6f06126e31..96419ab83d 100644 --- a/src/bro.bif +++ b/src/bro.bif @@ -440,13 +440,13 @@ static int do_system(const char* s) ## Invokes a command via the ``system`` function of the OS. ## The command runs in the background with ``stdout`` redirecting to ## ``stderr``. Here is a usage example: -## ``system(fmt("rm \"%s\"", str_shell_escape(sniffed_data)));`` +## ``system(fmt("rm %s", safe_shell_quote(sniffed_data)));`` ## ## str: The command to execute. ## ## Returns: The return value from the OS ``system`` function. ## -## .. bro:see:: system_env str_shell_escape piped_exec +## .. bro:see:: system_env safe_shell_quote piped_exec ## ## .. note:: ## @@ -472,7 +472,7 @@ function system%(str: string%): int ## ## Returns: The return value from the OS ``system`` function. ## -## .. bro:see:: system str_shell_escape piped_exec +## .. bro:see:: system safe_shell_quote piped_exec function system_env%(str: string, env: table_string_of_string%): int %{ if ( env->Type()->Tag() != TYPE_TABLE ) diff --git a/src/strings.bif b/src/strings.bif index 7435f5cffb..e7571d5c70 100644 --- a/src/strings.bif +++ b/src/strings.bif @@ -1184,10 +1184,54 @@ function string_fill%(len: int, source: string%): string ## ## source: The string to escape. ## +## Returns: A shell-escaped version of *source*. Specifically, this +## backslash-escapes characters whose literal value is not otherwise +## preserved by enclosure in double-quotes (dollar-sign, backquote, +## backslash, and double-quote itself), and then encloses that +## backslash-escaped string in double-quotes to ultimately preserve +## the literal value of all input characters. +## +## .. bro:see:: system safe_shell_quote +function safe_shell_quote%(source: string%): string + %{ + unsigned j = 0; + const u_char* src = source->Bytes(); + unsigned n = source->Len(); + byte_vec dst = new u_char[n * 2 + 1 + 2]; + dst[j++] = '"'; + + for ( unsigned i = 0; i < n; ++i ) + { + switch ( src[i] ) { + case '`': case '"': case '\\': case '$': + dst[j++] = '\\'; + break; + default: + break; + } + + dst[j++] = src[i]; + } + + dst[j++] = '"'; + dst[j] = '\0'; + return new StringVal(new BroString(1, dst, j)); + %} + +## Takes a string and escapes characters that would allow execution of +## commands at the shell level. Must be used before including strings in +## :bro:id:`system` or similar calls. This function is deprecated, use +## :bro:see:`safe_shell_quote` as a replacement. The difference is that +## :bro:see:`safe_shell_quote` automatically returns a value that is +## wrapped in double-quotes, which is required to correctly and fully +## escape any characters that might be interpreted by the shell. +## +## source: The string to escape. +## ## Returns: A shell-escaped version of *source*. ## -## .. bro:see:: system -function str_shell_escape%(source: string%): string +## .. bro:see:: system safe_shell_quote +function str_shell_escape%(source: string%): string &deprecated %{ unsigned j = 0; const u_char* src = source->Bytes(); diff --git a/testing/btest/Baseline/bifs.safe_shell_quote/out b/testing/btest/Baseline/bifs.safe_shell_quote/out new file mode 100644 index 0000000000..33e291680c --- /dev/null +++ b/testing/btest/Baseline/bifs.safe_shell_quote/out @@ -0,0 +1,2 @@ +echo `pwd` ${TEST} > "my file"; echo -e "\n" +"echo \`pwd\` \${TEST} > \"my file\"; echo -e \"\\n\"" diff --git a/testing/btest/bifs/safe_shell_quote.bro b/testing/btest/bifs/safe_shell_quote.bro new file mode 100644 index 0000000000..490952c79b --- /dev/null +++ b/testing/btest/bifs/safe_shell_quote.bro @@ -0,0 +1,12 @@ +# +# @TEST-EXEC: bro -b %INPUT >out +# @TEST-EXEC: btest-diff out + +event bro_init() + { + local a = "echo `pwd` ${TEST} > \"my file\"; echo -e \"\\n\""; + print a; + + local b = safe_shell_quote(a); + print b; + }