JS: UnsafeShellCommand Using unknown flags in the RegExp object is no longer flagged as bad sanitization to reduce false positives.

Author: Napalys

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(["'\\''", ""])
     }
   }

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 |

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 |

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.
 }