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

This commit is contained in:
Chris Smowton
2025-09-12 11:46:35 +01:00
parent e8ddac08b7
commit 4fb133a43d
2 changed files with 23 additions and 2 deletions

View File

@@ -33,6 +33,18 @@ ConditionGuardNode getLengthLEGuard(Variable index, Variable array) {
)
}
/**
* Gets a condition that checks that `index` is less than or equal to `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"

View File

@@ -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;
}