From 4ae25c2d34d7b3c3c72a47a3ea14d2b7d7f7a912 Mon Sep 17 00:00:00 2001 From: erik-krogh <erik-krogh@github.com> Date: Wed, 10 Apr 2024 14:26:00 +0200 Subject: [PATCH] don't mention arrays in the qhelp for rb/shell-command-constructed-from-input, because there are no array --- .../security/cwe-078/UnsafeShellCommandConstruction.qhelp | 7 ++++++- .../examples/unsafe-shell-command-construction_fixed.rb | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/ruby/ql/src/queries/security/cwe-078/UnsafeShellCommandConstruction.qhelp b/ruby/ql/src/queries/security/cwe-078/UnsafeShellCommandConstruction.qhelp index 4231f7cb0b40..8cedb57c277c 100644 --- a/ruby/ql/src/queries/security/cwe-078/UnsafeShellCommandConstruction.qhelp +++ b/ruby/ql/src/queries/security/cwe-078/UnsafeShellCommandConstruction.qhelp @@ -20,10 +20,15 @@ <recommendation> <p> - If possible, provide the dynamic arguments to the shell as an array + If possible, avoid concatenating shell strings to APIs such as <code>system(..)</code> to avoid interpretation by the shell. </p> + <p> + Instead, provide the arguments to the shell command as separate arguments to the + API, such as <code>system("echo", arg1, arg2)</code>. + </p> + <p> Alternatively, if the shell command must be constructed dynamically, then add code to ensure that special characters diff --git a/ruby/ql/src/queries/security/cwe-078/examples/unsafe-shell-command-construction_fixed.rb b/ruby/ql/src/queries/security/cwe-078/examples/unsafe-shell-command-construction_fixed.rb index cb8730eee099..b055f94eba99 100644 --- a/ruby/ql/src/queries/security/cwe-078/examples/unsafe-shell-command-construction_fixed.rb +++ b/ruby/ql/src/queries/security/cwe-078/examples/unsafe-shell-command-construction_fixed.rb @@ -1,6 +1,6 @@ module Utils def download(path) - # using an array to call `system` is safe + # using an API that doesn't interpret the path as a shell command system("wget", path) # OK end end \ No newline at end of file