JS: whitelist another emptiness check for the type-confusion query

Author: Esben Sparre Andreasen

change-notes/1.21/analysis-javascript.md

@@ -29,6 +29,7 @@
 | Incomplete string escaping or encoding | More results | This rule now considers the flow of regular expressions literals. |
 | Replacement of a substring with itself | More results | This rule now considers the flow of regular expressions literals. |
 | Server-side URL redirect       | Fewer false-positive results | This rule now treats URLs as safe in more cases where the hostname cannot be tampered with. |
+| Type confusion through parameter tampering | Fewer false-positive results | This rule now recognizes additional emptiness checks. |
 | Useless assignment to property | Fewer false-positive results | This rule now ignore reads of additional getters. |
 
 ## Changes to QL libraries

javascript/ql/src/semmle/javascript/security/dataflow/TypeConfusionThroughParameterTampering.qll

@@ -83,15 +83,25 @@ module TypeConfusionThroughParameterTampering {
     LengthAccess() {
       exists(DataFlow::PropRead read |
         read.accesses(this, "length") and
-        // exclude truthiness checks on the length: an array/string confusion cannot control an emptiness check
+        // an array/string confusion cannot control an emptiness check
         not (
+          // `if (x.length) {...}`
           exists(ConditionGuardNode cond | read.asExpr() = cond.getTest())
           or
+          // `x.length == 0`, `x.length > 0`
           exists(Comparison cmp, Expr zero |
             zero.getIntValue() = 0 and
             cmp.hasOperands(read.asExpr(), zero)
           )
           or
+          // `x.length < 1`
+          exists(RelationalComparison cmp |
+            cmp.getLesserOperand() = read.asExpr() and
+            cmp.getGreaterOperand().getIntValue() = 1 and
+            not cmp.isInclusive()
+          )
+          or
+          // `!x.length`
           exists(LogNotExpr neg | neg.getOperand() = read.asExpr())
         )
       )

javascript/ql/test/query-tests/Security/CWE-843/tst.js

@@ -68,4 +68,6 @@ express().get('/some/path/:foo', function(req, res) {
     while (p.length) { // OK
       p = p.substr(1);
     }
+
+    p.length < 1; // OK
 });