From 809af66e2429df0f69957e0c8075f7a394f0f96f Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Thu, 24 Sep 2026 13:33:15 +0100 Subject: [PATCH 1/3] Add test with duplicate results for go/index-out-of-bounds --- .../LengthComparisonOffByOne.expected | 4 ++++ .../InconsistentCode/LengthComparisonOffByOne/main.go | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/go/ql/test/query-tests/InconsistentCode/LengthComparisonOffByOne/LengthComparisonOffByOne.expected b/go/ql/test/query-tests/InconsistentCode/LengthComparisonOffByOne/LengthComparisonOffByOne.expected index 156b5860bd68..f1c9d849319a 100644 --- a/go/ql/test/query-tests/InconsistentCode/LengthComparisonOffByOne/LengthComparisonOffByOne.expected +++ b/go/ql/test/query-tests/InconsistentCode/LengthComparisonOffByOne/LengthComparisonOffByOne.expected @@ -1,3 +1,7 @@ | LengthComparisonOffByOne.go:8:14:8:29 | ...<=... | Off-by-one index comparison against length may lead to out-of-bounds $@. | LengthComparisonOffByOne.go:10:6:10:14 | index expression | read | | main.go:6:5:6:15 | ...<=... | Off-by-one index comparison against length may lead to out-of-bounds $@. | main.go:7:10:7:13 | index expression | read | | main.go:29:5:29:14 | ...>... | Off-by-one index comparison against length may lead to out-of-bounds $@. | main.go:30:10:30:13 | index expression | read | +| main.go:87:5:90:12 | !... | Off-by-one index comparison against length may lead to out-of-bounds $@. | main.go:93:9:93:12 | index expression | read | +| main.go:87:7:87:17 | ...>=... | Off-by-one index comparison against length may lead to out-of-bounds $@. | main.go:93:9:93:12 | index expression | read | +| main.go:87:7:88:11 | ...&&... | Off-by-one index comparison against length may lead to out-of-bounds $@. | main.go:93:9:93:12 | index expression | read | +| main.go:87:7:89:11 | ...&&... | Off-by-one index comparison against length may lead to out-of-bounds $@. | main.go:93:9:93:12 | index expression | read | diff --git a/go/ql/test/query-tests/InconsistentCode/LengthComparisonOffByOne/main.go b/go/ql/test/query-tests/InconsistentCode/LengthComparisonOffByOne/main.go index 01e849c0f2fc..df1147eb08b3 100644 --- a/go/ql/test/query-tests/InconsistentCode/LengthComparisonOffByOne/main.go +++ b/go/ql/test/query-tests/InconsistentCode/LengthComparisonOffByOne/main.go @@ -83,4 +83,14 @@ func f10(i int, a intintmap) int { return -1 } +func f11(a []int) int { + if !(len(a) >= 3 && // $ Alert + a[0] == 0 && // $ SPURIOUS: Alert + a[1] == 0 && // $ SPURIOUS: Alert + a[2] == 0) { // $ SPURIOUS: Alert + return -1 + } + return a[3] // $ Source +} + func main() {} From 19ff4073e56a54900fe922b48a49f5f8211d3154 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Thu, 24 Sep 2026 13:57:38 +0100 Subject: [PATCH 2/3] Fix dupes - report innermost condition --- go/ql/src/InconsistentCode/LengthComparisonOffByOne.ql | 6 ++++++ .../LengthComparisonOffByOne.expected | 3 --- .../InconsistentCode/LengthComparisonOffByOne/main.go | 6 +++--- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/go/ql/src/InconsistentCode/LengthComparisonOffByOne.ql b/go/ql/src/InconsistentCode/LengthComparisonOffByOne.ql index 176e34bc9bbb..808cb59393b3 100644 --- a/go/ql/src/InconsistentCode/LengthComparisonOffByOne.ql +++ b/go/ql/src/InconsistentCode/LengthComparisonOffByOne.ql @@ -87,6 +87,12 @@ where elementRead(ea, array, index, bb) and // and the read is guarded by the comparison cond.dominates(bb) and + // and report the innermost guard that establishes the comparison + not exists(ControlFlow::ConditionGuardNode innerCond | + innerCond = getLengthLEGuard(index, array) and + innerCond.dominates(bb) and + innerCond.getCondition().getParent+() = cond.getCondition() + ) and // but the read is not guarded by another check that `index != len(array)` not getLengthNEGuard(index, array).dominates(bb) and // and it is not additionally guarded by a stronger index check diff --git a/go/ql/test/query-tests/InconsistentCode/LengthComparisonOffByOne/LengthComparisonOffByOne.expected b/go/ql/test/query-tests/InconsistentCode/LengthComparisonOffByOne/LengthComparisonOffByOne.expected index f1c9d849319a..497e6fbf8e0e 100644 --- a/go/ql/test/query-tests/InconsistentCode/LengthComparisonOffByOne/LengthComparisonOffByOne.expected +++ b/go/ql/test/query-tests/InconsistentCode/LengthComparisonOffByOne/LengthComparisonOffByOne.expected @@ -1,7 +1,4 @@ | LengthComparisonOffByOne.go:8:14:8:29 | ...<=... | Off-by-one index comparison against length may lead to out-of-bounds $@. | LengthComparisonOffByOne.go:10:6:10:14 | index expression | read | | main.go:6:5:6:15 | ...<=... | Off-by-one index comparison against length may lead to out-of-bounds $@. | main.go:7:10:7:13 | index expression | read | | main.go:29:5:29:14 | ...>... | Off-by-one index comparison against length may lead to out-of-bounds $@. | main.go:30:10:30:13 | index expression | read | -| main.go:87:5:90:12 | !... | Off-by-one index comparison against length may lead to out-of-bounds $@. | main.go:93:9:93:12 | index expression | read | | main.go:87:7:87:17 | ...>=... | Off-by-one index comparison against length may lead to out-of-bounds $@. | main.go:93:9:93:12 | index expression | read | -| main.go:87:7:88:11 | ...&&... | Off-by-one index comparison against length may lead to out-of-bounds $@. | main.go:93:9:93:12 | index expression | read | -| main.go:87:7:89:11 | ...&&... | Off-by-one index comparison against length may lead to out-of-bounds $@. | main.go:93:9:93:12 | index expression | read | diff --git a/go/ql/test/query-tests/InconsistentCode/LengthComparisonOffByOne/main.go b/go/ql/test/query-tests/InconsistentCode/LengthComparisonOffByOne/main.go index df1147eb08b3..f0f1d6168638 100644 --- a/go/ql/test/query-tests/InconsistentCode/LengthComparisonOffByOne/main.go +++ b/go/ql/test/query-tests/InconsistentCode/LengthComparisonOffByOne/main.go @@ -85,9 +85,9 @@ func f10(i int, a intintmap) int { func f11(a []int) int { if !(len(a) >= 3 && // $ Alert - a[0] == 0 && // $ SPURIOUS: Alert - a[1] == 0 && // $ SPURIOUS: Alert - a[2] == 0) { // $ SPURIOUS: Alert + a[0] == 0 && + a[1] == 0 && + a[2] == 0) { return -1 } return a[3] // $ Source From 8c3c02948a01254f4334c52801652eab567dcaae Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Thu, 24 Sep 2026 14:02:30 +0100 Subject: [PATCH 3/3] Refactor query --- .../LengthComparisonOffByOne.ql | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/go/ql/src/InconsistentCode/LengthComparisonOffByOne.ql b/go/ql/src/InconsistentCode/LengthComparisonOffByOne.ql index 808cb59393b3..b36177fdc823 100644 --- a/go/ql/src/InconsistentCode/LengthComparisonOffByOne.ql +++ b/go/ql/src/InconsistentCode/LengthComparisonOffByOne.ql @@ -51,6 +51,13 @@ ControlFlow::ConditionGuardNode getLengthLEGuard(Index index, DataFlow::SsaNode ) } +predicate isDominatingLengthLEGuard( + ControlFlow::ConditionGuardNode guard, Index index, DataFlow::SsaNode array, BasicBlock bb +) { + guard = getLengthLEGuard(index, array) and + guard.dominates(bb) +} + /** * Gets a condition that checks that `index` is not equal to `array.length`. */ @@ -81,16 +88,13 @@ from ControlFlow::ConditionGuardNode cond, DataFlow::SsaNode array, Index index, DataFlow::ElementReadNode ea, BasicBlock bb where - // there is a comparison `index <= len(array)` - cond = getLengthLEGuard(index, array) and // there is a read from `array[index]` elementRead(ea, array, index, bb) and - // and the read is guarded by the comparison - cond.dominates(bb) and + // and it is guarded by a comparison `index <= len(array)` + isDominatingLengthLEGuard(cond, index, array, bb) and // and report the innermost guard that establishes the comparison not exists(ControlFlow::ConditionGuardNode innerCond | - innerCond = getLengthLEGuard(index, array) and - innerCond.dominates(bb) and + isDominatingLengthLEGuard(innerCond, index, array, bb) and innerCond.getCondition().getParent+() = cond.getCondition() ) and // but the read is not guarded by another check that `index != len(array)` @@ -99,7 +103,7 @@ where not exists(Index index2, int i, int i2 | index = ConstantIndex(i) and index2 = ConstantIndex(i2) and i < i2 | - getLengthLEGuard(index2, array).dominates(bb) + isDominatingLengthLEGuard(_, index2, array, bb) ) and not isRegexpMethodCall(array.getInit()) select cond.getCondition(),