diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionCustomizations.qll index 77625874df9f..8e753a5ef63b 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionCustomizations.qll @@ -245,7 +245,7 @@ module UnsafeShellCommandConstruction { class ReplaceQuotesSanitizer extends Sanitizer, StringReplaceCall { ReplaceQuotesSanitizer() { this.getAReplacedString() = "'" and - this.isGlobal() and + this.maybeGlobal() and this.getRawReplacement().mayHaveStringValue(["'\\''", ""]) } } diff --git a/javascript/ql/test/query-tests/Security/CWE-078/Consistency.expected b/javascript/ql/test/query-tests/Security/CWE-078/Consistency.expected index 5844d8d221de..e69de29bb2d1 100644 --- a/javascript/ql/test/query-tests/Security/CWE-078/Consistency.expected +++ b/javascript/ql/test/query-tests/Security/CWE-078/Consistency.expected @@ -1 +0,0 @@ -| UnsafeShellCommandConstruction/lib/lib.js:640 | did not expect an alert, but found an alert for UnsafeShellCommandConstruction | OK -- Currently this is flagged as a bad sanitization, but it is not certain that it is bad. | ComandInjection | diff --git a/javascript/ql/test/query-tests/Security/CWE-078/UnsafeShellCommandConstruction/UnsafeShellCommandConstruction.expected b/javascript/ql/test/query-tests/Security/CWE-078/UnsafeShellCommandConstruction/UnsafeShellCommandConstruction.expected index 83c4a4718546..f2fa354a3050 100644 --- a/javascript/ql/test/query-tests/Security/CWE-078/UnsafeShellCommandConstruction/UnsafeShellCommandConstruction.expected +++ b/javascript/ql/test/query-tests/Security/CWE-078/UnsafeShellCommandConstruction/UnsafeShellCommandConstruction.expected @@ -328,13 +328,6 @@ nodes | lib/lib.js:633:24:633:62 | name.re ... '\\\\''") | | lib/lib.js:634:22:634:30 | sanitized | | lib/lib.js:634:22:634:30 | sanitized | -| lib/lib.js:639:6:639:84 | sanitized | -| lib/lib.js:639:18:639:84 | "'" + n ... ) + "'" | -| lib/lib.js:639:24:639:27 | name | -| lib/lib.js:639:24:639:78 | name.re ... '\\\\''") | -| lib/lib.js:639:24:639:78 | name.re ... '\\\\''") | -| lib/lib.js:640:22:640:30 | sanitized | -| lib/lib.js:640:22:640:30 | sanitized | | lib/subLib2/compiled-file.ts:3:26:3:29 | name | | lib/subLib2/compiled-file.ts:3:26:3:29 | name | | lib/subLib2/compiled-file.ts:4:25:4:28 | name | @@ -767,20 +760,12 @@ edges | lib/lib.js:608:42:608:45 | name | lib/lib.js:629:25:629:28 | name | | lib/lib.js:632:38:632:41 | name | lib/lib.js:633:24:633:27 | name | | lib/lib.js:632:38:632:41 | name | lib/lib.js:633:24:633:27 | name | -| lib/lib.js:632:38:632:41 | name | lib/lib.js:639:24:639:27 | name | -| lib/lib.js:632:38:632:41 | name | lib/lib.js:639:24:639:27 | name | | lib/lib.js:633:6:633:68 | sanitized | lib/lib.js:634:22:634:30 | sanitized | | lib/lib.js:633:6:633:68 | sanitized | lib/lib.js:634:22:634:30 | sanitized | | lib/lib.js:633:18:633:68 | "'" + n ... ) + "'" | lib/lib.js:633:6:633:68 | sanitized | | lib/lib.js:633:24:633:27 | name | lib/lib.js:633:24:633:62 | name.re ... '\\\\''") | | lib/lib.js:633:24:633:27 | name | lib/lib.js:633:24:633:62 | name.re ... '\\\\''") | | lib/lib.js:633:24:633:62 | name.re ... '\\\\''") | lib/lib.js:633:18:633:68 | "'" + n ... ) + "'" | -| lib/lib.js:639:6:639:84 | sanitized | lib/lib.js:640:22:640:30 | sanitized | -| lib/lib.js:639:6:639:84 | sanitized | lib/lib.js:640:22:640:30 | sanitized | -| lib/lib.js:639:18:639:84 | "'" + n ... ) + "'" | lib/lib.js:639:6:639:84 | sanitized | -| lib/lib.js:639:24:639:27 | name | lib/lib.js:639:24:639:78 | name.re ... '\\\\''") | -| lib/lib.js:639:24:639:27 | name | lib/lib.js:639:24:639:78 | name.re ... '\\\\''") | -| lib/lib.js:639:24:639:78 | name.re ... '\\\\''") | lib/lib.js:639:18:639:84 | "'" + n ... ) + "'" | | lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name | | lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name | | lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name | @@ -913,8 +898,6 @@ edges | lib/lib.js:629:13:629:28 | "rm -rf " + name | lib/lib.js:608:42:608:45 | name | lib/lib.js:629:25:629:28 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:608:42:608:45 | name | library input | lib/lib.js:629:5:629:29 | cp.exec ... + name) | shell command | | lib/lib.js:633:18:633:68 | "'" + n ... ) + "'" | lib/lib.js:632:38:632:41 | name | lib/lib.js:633:24:633:62 | name.re ... '\\\\''") | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:632:38:632:41 | name | library input | lib/lib.js:634:2:634:31 | cp.exec ... itized) | shell command | | lib/lib.js:634:10:634:30 | "rm -rf ... nitized | lib/lib.js:632:38:632:41 | name | lib/lib.js:634:22:634:30 | sanitized | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:632:38:632:41 | name | library input | lib/lib.js:634:2:634:31 | cp.exec ... itized) | shell command | -| lib/lib.js:639:18:639:84 | "'" + n ... ) + "'" | lib/lib.js:632:38:632:41 | name | lib/lib.js:639:24:639:78 | name.re ... '\\\\''") | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:632:38:632:41 | name | library input | lib/lib.js:640:2:640:31 | cp.exec ... itized) | shell command | -| lib/lib.js:640:10:640:30 | "rm -rf ... nitized | lib/lib.js:632:38:632:41 | name | lib/lib.js:640:22:640:30 | sanitized | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:632:38:632:41 | name | library input | lib/lib.js:640:2:640:31 | cp.exec ... itized) | shell command | | lib/subLib2/compiled-file.ts:4:13:4:28 | "rm -rf " + name | lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name | This string concatenation which depends on $@ is later used in a $@. | lib/subLib2/compiled-file.ts:3:26:3:29 | name | library input | lib/subLib2/compiled-file.ts:4:5:4:29 | cp.exec ... + name) | shell command | | lib/subLib2/special-file.js:4:10:4:25 | "rm -rf " + name | lib/subLib2/special-file.js:3:28:3:31 | name | lib/subLib2/special-file.js:4:22:4:25 | name | This string concatenation which depends on $@ is later used in a $@. | lib/subLib2/special-file.js:3:28:3:31 | name | library input | lib/subLib2/special-file.js:4:2:4:26 | cp.exec ... + name) | shell command | | lib/subLib3/my-file.ts:4:10:4:25 | "rm -rf " + name | lib/subLib3/my-file.ts:3:28:3:31 | name | lib/subLib3/my-file.ts:4:22:4:25 | name | This string concatenation which depends on $@ is later used in a $@. | lib/subLib3/my-file.ts:3:28:3:31 | name | library input | lib/subLib3/my-file.ts:4:2:4:26 | cp.exec ... + name) | shell command | diff --git a/javascript/ql/test/query-tests/Security/CWE-078/UnsafeShellCommandConstruction/lib/lib.js b/javascript/ql/test/query-tests/Security/CWE-078/UnsafeShellCommandConstruction/lib/lib.js index d3190ceb6462..09488f0a8871 100644 --- a/javascript/ql/test/query-tests/Security/CWE-078/UnsafeShellCommandConstruction/lib/lib.js +++ b/javascript/ql/test/query-tests/Security/CWE-078/UnsafeShellCommandConstruction/lib/lib.js @@ -637,5 +637,5 @@ module.exports.sanitizer = function (name) { cp.exec("rm -rf " + sanitized); // OK var sanitized = "'" + name.replace(new RegExp("\'", unknownFlags()), "'\\''") + "'" - cp.exec("rm -rf " + sanitized); // OK -- Currently this is flagged as a bad sanitization, but it is not certain that it is bad. + cp.exec("rm -rf " + sanitized); // OK -- Most likely should be okay and not flagged to reduce false positives. }