support more patterns that recognize valid numbers

Author: erik-krogh

javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll

@@ -1179,6 +1179,17 @@ module TaintTracking {
         parse = isNaN.getArgument(0) and
         operand = parse.getArgument(0).asExpr()
       )
+      or
+      exists(UnaryExpr unary | unary.getOperator() = ["+", "-"] |
+        unary = isNaN.getArgument(0).asExpr() and
+        operand = unary.getOperand()
+      )
+      or
+      exists(BinaryExpr bin | bin.getOperator() = ["+", "-"] |
+        bin = isNaN.getArgument(0).asExpr() and
+        operand = bin.getAnOperand() and
+        bin.getAnOperand() instanceof NumberLiteral
+      )
     )
     or
     isTypeofGuard(guard.asExpr(), operand, "number") and

javascript/ql/test/query-tests/Security/CWE-078/UnsafeShellCommandConstruction.expected

@@ -262,6 +262,16 @@ nodes
 | lib/lib.js:513:23:513:26 | name |
 | lib/lib.js:519:23:519:26 | name |
 | lib/lib.js:519:23:519:26 | name |
+| lib/lib.js:525:23:525:26 | name |
+| lib/lib.js:525:23:525:26 | name |
+| lib/lib.js:531:23:531:26 | name |
+| lib/lib.js:531:23:531:26 | name |
+| lib/lib.js:537:23:537:26 | name |
+| lib/lib.js:537:23:537:26 | name |
+| lib/lib.js:543:23:543:26 | name |
+| lib/lib.js:543:23:543:26 | name |
+| lib/lib.js:545:23:545:26 | name |
+| lib/lib.js:545:23:545:26 | name |
 | 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 |
@@ -594,6 +604,26 @@ edges
 | lib/lib.js:509:39:509:42 | name | lib/lib.js:519:23:519:26 | name |
 | lib/lib.js:509:39:509:42 | name | lib/lib.js:519:23:519:26 | name |
 | lib/lib.js:509:39:509:42 | name | lib/lib.js:519:23:519:26 | name |
+| lib/lib.js:509:39:509:42 | name | lib/lib.js:525:23:525:26 | name |
+| lib/lib.js:509:39:509:42 | name | lib/lib.js:525:23:525:26 | name |
+| lib/lib.js:509:39:509:42 | name | lib/lib.js:525:23:525:26 | name |
+| lib/lib.js:509:39:509:42 | name | lib/lib.js:525:23:525:26 | name |
+| lib/lib.js:509:39:509:42 | name | lib/lib.js:531:23:531:26 | name |
+| lib/lib.js:509:39:509:42 | name | lib/lib.js:531:23:531:26 | name |
+| lib/lib.js:509:39:509:42 | name | lib/lib.js:531:23:531:26 | name |
+| lib/lib.js:509:39:509:42 | name | lib/lib.js:531:23:531:26 | name |
+| lib/lib.js:509:39:509:42 | name | lib/lib.js:537:23:537:26 | name |
+| lib/lib.js:509:39:509:42 | name | lib/lib.js:537:23:537:26 | name |
+| lib/lib.js:509:39:509:42 | name | lib/lib.js:537:23:537:26 | name |
+| lib/lib.js:509:39:509:42 | name | lib/lib.js:537:23:537:26 | name |
+| lib/lib.js:509:39:509:42 | name | lib/lib.js:543:23:543:26 | name |
+| lib/lib.js:509:39:509:42 | name | lib/lib.js:543:23:543:26 | name |
+| lib/lib.js:509:39:509:42 | name | lib/lib.js:543:23:543:26 | name |
+| lib/lib.js:509:39:509:42 | name | lib/lib.js:543:23:543:26 | name |
+| lib/lib.js:509:39:509:42 | name | lib/lib.js:545:23:545:26 | name |
+| lib/lib.js:509:39:509:42 | name | lib/lib.js:545:23:545:26 | name |
+| lib/lib.js:509:39:509:42 | name | lib/lib.js:545:23:545:26 | name |
+| lib/lib.js:509:39:509:42 | name | lib/lib.js:545:23:545:26 | 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 |
 | lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name |
@@ -689,6 +719,11 @@ edges
 | lib/lib.js:510:10:510:25 | "rm -rf " + name | lib/lib.js:509:39:509:42 | name | lib/lib.js:510:22:510:25 | name | $@ based on $@ is later used in $@. | lib/lib.js:510:10:510:25 | "rm -rf " + name | String concatenation | lib/lib.js:509:39:509:42 | name | library input | lib/lib.js:510:2:510:26 | cp.exec ... + name) | shell command |
 | lib/lib.js:513:11:513:26 | "rm -rf " + name | lib/lib.js:509:39:509:42 | name | lib/lib.js:513:23:513:26 | name | $@ based on $@ is later used in $@. | lib/lib.js:513:11:513:26 | "rm -rf " + name | String concatenation | lib/lib.js:509:39:509:42 | name | library input | lib/lib.js:513:3:513:27 | cp.exec ... + name) | shell command |
 | lib/lib.js:519:11:519:26 | "rm -rf " + name | lib/lib.js:509:39:509:42 | name | lib/lib.js:519:23:519:26 | name | $@ based on $@ is later used in $@. | lib/lib.js:519:11:519:26 | "rm -rf " + name | String concatenation | lib/lib.js:509:39:509:42 | name | library input | lib/lib.js:519:3:519:27 | cp.exec ... + name) | shell command |
+| lib/lib.js:525:11:525:26 | "rm -rf " + name | lib/lib.js:509:39:509:42 | name | lib/lib.js:525:23:525:26 | name | $@ based on $@ is later used in $@. | lib/lib.js:525:11:525:26 | "rm -rf " + name | String concatenation | lib/lib.js:509:39:509:42 | name | library input | lib/lib.js:525:3:525:27 | cp.exec ... + name) | shell command |
+| lib/lib.js:531:11:531:26 | "rm -rf " + name | lib/lib.js:509:39:509:42 | name | lib/lib.js:531:23:531:26 | name | $@ based on $@ is later used in $@. | lib/lib.js:531:11:531:26 | "rm -rf " + name | String concatenation | lib/lib.js:509:39:509:42 | name | library input | lib/lib.js:531:3:531:27 | cp.exec ... + name) | shell command |
+| lib/lib.js:537:11:537:26 | "rm -rf " + name | lib/lib.js:509:39:509:42 | name | lib/lib.js:537:23:537:26 | name | $@ based on $@ is later used in $@. | lib/lib.js:537:11:537:26 | "rm -rf " + name | String concatenation | lib/lib.js:509:39:509:42 | name | library input | lib/lib.js:537:3:537:27 | cp.exec ... + name) | shell command |
+| lib/lib.js:543:11:543:26 | "rm -rf " + name | lib/lib.js:509:39:509:42 | name | lib/lib.js:543:23:543:26 | name | $@ based on $@ is later used in $@. | lib/lib.js:543:11:543:26 | "rm -rf " + name | String concatenation | lib/lib.js:509:39:509:42 | name | library input | lib/lib.js:543:3:543:27 | cp.exec ... + name) | shell command |
+| lib/lib.js:545:11:545:26 | "rm -rf " + name | lib/lib.js:509:39:509:42 | name | lib/lib.js:545:23:545:26 | name | $@ based on $@ is later used in $@. | lib/lib.js:545:11:545:26 | "rm -rf " + name | String concatenation | lib/lib.js:509:39:509:42 | name | library input | lib/lib.js:545:3:545:27 | cp.exec ... + name) | 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 | $@ based on $@ is later used in $@. | lib/subLib2/compiled-file.ts:4:13:4:28 | "rm -rf " + name | String concatenation | 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 | $@ based on $@ is later used in $@. | lib/subLib2/special-file.js:4:10:4:25 | "rm -rf " + name | String concatenation | 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 | $@ based on $@ is later used in $@. | lib/subLib3/my-file.ts:4:10:4:25 | "rm -rf " + name | String concatenation | 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/lib/lib.js

@@ -520,4 +520,28 @@ module.exports.sanitizer4 = function (name) {
 	} else {
 		cp.exec("rm -rf " + name); // OK
 	}
+
+	if (isNaN(+name)) {
+		cp.exec("rm -rf " + name); // NOT OK
+	} else {
+		cp.exec("rm -rf " + name); // OK
+	}
+
+	if (isNaN(parseInt(name, 10))) {
+		cp.exec("rm -rf " + name); // NOT OK
+	} else {
+		cp.exec("rm -rf " + name); // OK
+	}
+
+	if (isNaN(name - 0)) {
+		cp.exec("rm -rf " + name); // NOT OK
+	} else {
+		cp.exec("rm -rf " + name); // OK
+	}
+
+	if (isNaN(name | 0)) { // <- not a sanitizer
+		cp.exec("rm -rf " + name); // NOT OK
+	} else {
+		cp.exec("rm -rf " + name); // NOT OK
+	}
 }