Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions go/ql/src/InconsistentCode/LengthComparisonOffByOne.ql
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
*/
Expand Down Expand Up @@ -81,19 +88,22 @@ 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 |
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)`
not getLengthNEGuard(index, array).dominates(bb) and
// and it is not additionally guarded by a stronger index check
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(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +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: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 |
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
a[1] == 0 &&
a[2] == 0) {
return -1
}
return a[3] // $ Source
}

func main() {}
Loading