CPP: Check for a range check before the use.

Author: geoffw0

cpp/ql/src/Best Practices/Likely Errors/OffsetUseBeforeRangeCheck.ql

@@ -13,10 +13,29 @@
 
 import cpp
 
-from Variable v, LogicalAndExpr andexpr, ArrayExpr access, LTExpr rangecheck
-where access.getArrayOffset() = v.getAnAccess()
-  and andexpr.getLeftOperand().getAChild*() = access
-  and andexpr.getRightOperand() = rangecheck
-  and rangecheck.getLeftOperand() = v.getAnAccess()
-  and not access.isInMacroExpansion()
+predicate beforeArrayAccess(Variable v, ArrayExpr access, Expr before) {
+  exists(LogicalAndExpr andexpr |
+    access.getArrayOffset() = v.getAnAccess() and
+    andexpr.getRightOperand().getAChild*() = access and
+    andexpr.getLeftOperand() = before
+  )
+}
+
+predicate afterArrayAccess(Variable v, ArrayExpr access, Expr after) {
+  exists(LogicalAndExpr andexpr |
+    access.getArrayOffset() = v.getAnAccess() and
+    andexpr.getLeftOperand().getAChild*() = access and
+    andexpr.getRightOperand() = after
+  )
+}
+
+from Variable v, ArrayExpr access, LTExpr rangecheck
+where
+  afterArrayAccess(v, access, rangecheck) and
+  rangecheck.getLeftOperand() = v.getAnAccess() and
+  not access.isInMacroExpansion() and
+  not exists(LTExpr altcheck |
+    beforeArrayAccess(v, access, altcheck) and
+    altcheck.getLeftOperand() = v.getAnAccess()
+  )
 select access, "This use of offset '" + v.getName() + "' should follow the $@.", rangecheck, "range check"

cpp/ql/test/query-tests/Best Practices/Likely Errors/OffsetUseBeforeRangeCheck/OffsetUseBeforeRangeCheck.expected

@@ -1,10 +1,6 @@
 | test.cpp:11:10:11:18 | access to array | This use of offset 'i' should follow the $@. | test.cpp:11:32:11:45 | ... < ... | range check |
 | test.cpp:15:7:15:15 | access to array | This use of offset 'i' should follow the $@. | test.cpp:15:29:15:42 | ... < ... | range check |
 | test.cpp:27:7:27:15 | access to array | This use of offset 'i' should follow the $@. | test.cpp:27:39:27:52 | ... < ... | range check |
-| test.cpp:32:27:32:35 | access to array | This use of offset 'i' should follow the $@. | test.cpp:32:49:32:66 | ... < ... | range check |
-| test.cpp:33:28:33:36 | access to array | This use of offset 'i' should follow the $@. | test.cpp:33:50:33:67 | ... < ... | range check |
-| test.cpp:34:31:34:39 | access to array | This use of offset 'i' should follow the $@. | test.cpp:34:53:34:66 | ... < ... | range check |
-| test.cpp:35:32:35:40 | access to array | This use of offset 'i' should follow the $@. | test.cpp:35:54:35:67 | ... < ... | range check |
 | test.cpp:39:8:39:16 | access to array | This use of offset 'i' should follow the $@. | test.cpp:39:30:39:47 | ... < ... | range check |
 | test.cpp:44:7:44:15 | access to array | This use of offset 'i' should follow the $@. | test.cpp:44:22:44:35 | ... < ... | range check |
 | test.cpp:47:7:47:15 | access to array | This use of offset 'i' should follow the $@. | test.cpp:47:33:47:46 | ... < ... | range check |

cpp/ql/test/query-tests/Best Practices/Likely Errors/OffsetUseBeforeRangeCheck/test.cpp

@@ -29,10 +29,10 @@ void test(char *buffer, int bufferSize)
 	if ((i < bufferSize - 1) && (buffer[i + 1] == 'x')) {} // GOOD
 	if ((buffer[i + 1] == 'x') && (i < bufferSize - 1)) {} // BAD [NOT DETECTED]
 
-	if ((i < bufferSize) && (buffer[i] == 'x') && (i < bufferSize - 1)) {} // GOOD [FALSE POSITIVE]
-	if ((i < bufferSize) && ((buffer[i] == 'x') && (i < bufferSize - 1))) {} // GOOD [FALSE POSITIVE]
-	if ((i < bufferSize + 1) && (buffer[i] == 'x') && (i < bufferSize)) {} // BAD
-	if ((i < bufferSize + 1) && ((buffer[i] == 'x') && (i < bufferSize))) {} // BAD
+	if ((i < bufferSize) && (buffer[i] == 'x') && (i < bufferSize - 1)) {} // GOOD
+	if ((i < bufferSize) && ((buffer[i] == 'x') && (i < bufferSize - 1))) {} // GOOD
+	if ((i < bufferSize + 1) && (buffer[i] == 'x') && (i < bufferSize)) {} // BAD [NOT DETECTED]
+	if ((i < bufferSize + 1) && ((buffer[i] == 'x') && (i < bufferSize))) {} // BAD [NOT DETECTED]
 
 	// look for 'ab'
 	for (i = 0; i < bufferSize; i++) {