Merge pull request #20423 from smowton/smowton/fix/length-comparison-off-by-one-fp

JS: Recognise that a less-than test is as good as a non-equal test for mitigating off-by-one array access

javascript/ql/src/LanguageFeatures/LengthComparisonOffByOne.ql

@@ -33,6 +33,18 @@ ConditionGuardNode getLengthLEGuard(Variable index, Variable array) {
   )
 }
 
+/**
+ * Gets a condition that checks that `index` is less than `array.length`.
+ */
+ConditionGuardNode getLengthLTGuard(Variable index, Variable array) {
+  exists(RelationalComparison cmp | cmp instanceof GTExpr or cmp instanceof LTExpr |
+    cmp = result.getTest() and
+    result.getOutcome() = true and
+    cmp.getGreaterOperand() = arrayLen(array) and
+    cmp.getLesserOperand() = index.getAnAccess()
+  )
+}
+
 /**
  * Gets a condition that checks that `index` is not equal to `array.length`.
  */
@@ -62,7 +74,8 @@ where
   elementRead(ea, array, index, bb) and
   // and the read is guarded by the comparison
   cond.dominates(bb) and
-  // but the read is not guarded by another check that `index != array.length`
-  not getLengthNEGuard(index, array).dominates(bb)
+  // but the read is not guarded by another check that `index != array.length` or `index < array.length`
+  not getLengthNEGuard(index, array).dominates(bb) and
+  not getLengthLTGuard(index, array).dominates(bb)
 select cond.getTest(), "Off-by-one index comparison against length may lead to out-of-bounds $@.",
   ea, "read"

javascript/ql/src/change-notes/2025-09-12-off-by-one.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Query `js/index-out-of-bounds` no longer produces a false-positive when a strictly-less-than check overrides a previous less-than-or-equal test.

javascript/ql/test/query-tests/LanguageFeatures/LengthComparisonOffByOne/tst.js

@@ -55,3 +55,11 @@ function badContains(a, elt) {
       return true;
   return false;
 }
+
+// OK - incorrect upper bound, but extra check
+function badContains2(a, elt) {
+  for (let i = 0; i <= a.length; ++i)
+    if (i < a.length && a[i] === elt)
+      return true;
+  return false;
+}