diff --git a/javascript/ql/src/LanguageFeatures/LengthComparisonOffByOne.ql b/javascript/ql/src/LanguageFeatures/LengthComparisonOffByOne.ql index 4f3815d6fc6b..c5766d61cd6e 100644 --- a/javascript/ql/src/LanguageFeatures/LengthComparisonOffByOne.ql +++ b/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" diff --git a/javascript/ql/src/change-notes/2025-09-12-off-by-one.md b/javascript/ql/src/change-notes/2025-09-12-off-by-one.md new file mode 100644 index 000000000000..42a97195d4f3 --- /dev/null +++ b/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. diff --git a/javascript/ql/test/query-tests/LanguageFeatures/LengthComparisonOffByOne/tst.js b/javascript/ql/test/query-tests/LanguageFeatures/LengthComparisonOffByOne/tst.js index 6b214c9b0423..bd284d2c0276 100644 --- a/javascript/ql/test/query-tests/LanguageFeatures/LengthComparisonOffByOne/tst.js +++ b/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; +}