Merge pull request #22678 from owen-mc/go/fix/length-comparison-duplicates

Go: fix some duplicate results in `go/index-out-of-bounds`

Author: owen-mc

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,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(),

go/ql/test/query-tests/InconsistentCode/LengthComparisonOffByOne/LengthComparisonOffByOne.expected

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

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 &&
+		a[1] == 0 &&
+		a[2] == 0) {
+		return -1
+	}
+	return a[3] // $ Source
+}
+
 func main() {}