From b7653ec92ddd617bf8745643cb7e6d32cd27a1aa Mon Sep 17 00:00:00 2001 From: Robert Marsh Date: Thu, 4 May 2023 16:39:02 -0400 Subject: [PATCH] C++: ignore cast arrays in constant off-by-one query --- .../Security/CWE/CWE-193/ConstantSizeArrayOffByOne.ql | 1 + .../constant-size/ConstantSizeArrayOffByOne.expected | 6 ------ .../query-tests/Security/CWE/CWE-193/constant-size/test.cpp | 4 ++-- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-193/ConstantSizeArrayOffByOne.ql b/cpp/ql/src/experimental/Security/CWE/CWE-193/ConstantSizeArrayOffByOne.ql index af41bb7222a..ce604510d70 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-193/ConstantSizeArrayOffByOne.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-193/ConstantSizeArrayOffByOne.ql @@ -98,6 +98,7 @@ predicate isConstantSizeOverflowSource(Field f, FieldAddressToPointerArithmeticF FieldAddressToPointerArithmeticFlow::flowPath(fieldSource, sink) and isFieldAddressSource(f, fieldSource.getNode()) and pai.getLeft() = sink.getNode().(DataFlow::InstructionNode).asInstruction() and + pai.getElementSize() = f.getUnspecifiedType().(ArrayType).getBaseType().getSize() and f.getUnspecifiedType().(ArrayType).getArraySize() = size and semBounded(getSemanticExpr(pai.getRight()), any(SemZeroBound b), bound, true, _) and delta = bound - size and diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/constant-size/ConstantSizeArrayOffByOne.expected b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/constant-size/ConstantSizeArrayOffByOne.expected index 294ddb0e46d..0b688810262 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/constant-size/ConstantSizeArrayOffByOne.expected +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/constant-size/ConstantSizeArrayOffByOne.expected @@ -35,8 +35,6 @@ edges | test.cpp:79:32:79:34 | buf | test.cpp:79:27:79:34 | buf | | test.cpp:85:34:85:36 | buf | test.cpp:87:5:87:11 | charBuf | | test.cpp:85:34:85:36 | buf | test.cpp:88:5:88:11 | charBuf | -| test.cpp:87:5:87:11 | charBuf | test.cpp:87:5:87:31 | access to array | -| test.cpp:88:5:88:11 | charBuf | test.cpp:88:5:88:27 | access to array | nodes | test.cpp:26:5:26:12 | buf | semmle.label | buf | | test.cpp:26:10:26:12 | buf | semmle.label | buf | @@ -90,9 +88,7 @@ nodes | test.cpp:79:32:79:34 | buf | semmle.label | buf | | test.cpp:85:34:85:36 | buf | semmle.label | buf | | test.cpp:87:5:87:11 | charBuf | semmle.label | charBuf | -| test.cpp:87:5:87:31 | access to array | semmle.label | access to array | | test.cpp:88:5:88:11 | charBuf | semmle.label | charBuf | -| test.cpp:88:5:88:27 | access to array | semmle.label | access to array | subpaths #select | test.cpp:35:5:35:22 | access to array | test.cpp:35:10:35:12 | buf | test.cpp:35:5:35:22 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:35:5:35:26 | Store: ... = ... | write | @@ -107,5 +103,3 @@ subpaths | test.cpp:77:27:77:44 | access to array | test.cpp:77:32:77:34 | buf | test.cpp:66:32:66:32 | p | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:67:5:67:10 | Store: ... = ... | write | | test.cpp:77:27:77:44 | access to array | test.cpp:77:32:77:34 | buf | test.cpp:67:5:67:6 | * ... | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:67:5:67:10 | Store: ... = ... | write | | test.cpp:77:27:77:44 | access to array | test.cpp:77:32:77:34 | buf | test.cpp:67:6:67:6 | p | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:67:5:67:10 | Store: ... = ... | write | -| test.cpp:87:5:87:31 | access to array | test.cpp:85:34:85:36 | buf | test.cpp:87:5:87:31 | access to array | This pointer arithmetic may have an off-by-3072 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:87:5:87:35 | Store: ... = ... | write | -| test.cpp:88:5:88:27 | access to array | test.cpp:85:34:85:36 | buf | test.cpp:88:5:88:27 | access to array | This pointer arithmetic may have an off-by-3073 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:88:5:88:31 | Store: ... = ... | write | diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/constant-size/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/constant-size/test.cpp index c2ca2401127..5749331b7d5 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/constant-size/test.cpp +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/constant-size/test.cpp @@ -84,6 +84,6 @@ void testInterproc(BigArray *arr) { void testCharIndex(BigArray *arr) { char *charBuf = (char*) arr->buf; - charBuf[MAX_SIZE_BYTES - 1] = 0; // GOOD [FALSE POSITIVE] - charBuf[MAX_SIZE_BYTES] = 0; // BAD + charBuf[MAX_SIZE_BYTES - 1] = 0; // GOOD + charBuf[MAX_SIZE_BYTES] = 0; // BAD [FALSE NEGATIVE] } \ No newline at end of file