From 90f0209095943c7d1c9b1887a682cab03a8b5d95 Mon Sep 17 00:00:00 2001 From: Jeroen Ketema Date: Mon, 5 Jun 2023 14:53:01 +0200 Subject: [PATCH 1/3] C++: Add `cpp/invalid-pointer-deref` test case with almost duplicated results --- .../InvalidPointerDeref.expected | 36 +++++++++++++++++++ .../CWE/CWE-193/pointer-deref/test.cpp | 11 ++++++ 2 files changed, 47 insertions(+) diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/pointer-deref/InvalidPointerDeref.expected b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/pointer-deref/InvalidPointerDeref.expected index 09c75e7369c..f13d7fbf86a 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/pointer-deref/InvalidPointerDeref.expected +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/pointer-deref/InvalidPointerDeref.expected @@ -732,6 +732,29 @@ edges | test.cpp:368:5:368:10 | ... += ... | test.cpp:372:16:372:16 | p | | test.cpp:371:7:371:7 | p | test.cpp:372:15:372:16 | Load: * ... | | test.cpp:372:16:372:16 | p | test.cpp:372:15:372:16 | Load: * ... | +| test.cpp:377:14:377:27 | new[] | test.cpp:378:15:378:16 | xs | +| test.cpp:378:15:378:16 | xs | test.cpp:378:15:378:23 | ... + ... | +| test.cpp:378:15:378:16 | xs | test.cpp:378:15:378:23 | ... + ... | +| test.cpp:378:15:378:16 | xs | test.cpp:378:15:378:23 | ... + ... | +| test.cpp:378:15:378:16 | xs | test.cpp:378:15:378:23 | ... + ... | +| test.cpp:378:15:378:16 | xs | test.cpp:381:5:381:7 | end | +| test.cpp:378:15:378:16 | xs | test.cpp:381:5:381:9 | ... ++ | +| test.cpp:378:15:378:16 | xs | test.cpp:381:5:381:9 | ... ++ | +| test.cpp:378:15:378:16 | xs | test.cpp:384:14:384:16 | end | +| test.cpp:378:15:378:23 | ... + ... | test.cpp:378:15:378:23 | ... + ... | +| test.cpp:378:15:378:23 | ... + ... | test.cpp:378:15:378:23 | ... + ... | +| test.cpp:378:15:378:23 | ... + ... | test.cpp:381:5:381:7 | end | +| test.cpp:378:15:378:23 | ... + ... | test.cpp:381:5:381:7 | end | +| test.cpp:378:15:378:23 | ... + ... | test.cpp:384:13:384:16 | Load: * ... | +| test.cpp:378:15:378:23 | ... + ... | test.cpp:384:13:384:16 | Load: * ... | +| test.cpp:378:15:378:23 | ... + ... | test.cpp:384:13:384:16 | Load: * ... | +| test.cpp:378:15:378:23 | ... + ... | test.cpp:384:13:384:16 | Load: * ... | +| test.cpp:378:15:378:23 | ... + ... | test.cpp:384:14:384:16 | end | +| test.cpp:378:15:378:23 | ... + ... | test.cpp:384:14:384:16 | end | +| test.cpp:381:5:381:7 | end | test.cpp:384:13:384:16 | Load: * ... | +| test.cpp:381:5:381:9 | ... ++ | test.cpp:384:14:384:16 | end | +| test.cpp:381:5:381:9 | ... ++ | test.cpp:384:14:384:16 | end | +| test.cpp:384:14:384:16 | end | test.cpp:384:13:384:16 | Load: * ... | nodes | test.cpp:4:15:4:20 | call to malloc | semmle.label | call to malloc | | test.cpp:5:15:5:15 | p | semmle.label | p | @@ -1066,6 +1089,17 @@ nodes | test.cpp:371:7:371:7 | p | semmle.label | p | | test.cpp:372:15:372:16 | Load: * ... | semmle.label | Load: * ... | | test.cpp:372:16:372:16 | p | semmle.label | p | +| test.cpp:377:14:377:27 | new[] | semmle.label | new[] | +| test.cpp:378:15:378:16 | xs | semmle.label | xs | +| test.cpp:378:15:378:23 | ... + ... | semmle.label | ... + ... | +| test.cpp:378:15:378:23 | ... + ... | semmle.label | ... + ... | +| test.cpp:378:15:378:23 | ... + ... | semmle.label | ... + ... | +| test.cpp:378:15:378:23 | ... + ... | semmle.label | ... + ... | +| test.cpp:381:5:381:7 | end | semmle.label | end | +| test.cpp:381:5:381:9 | ... ++ | semmle.label | ... ++ | +| test.cpp:381:5:381:9 | ... ++ | semmle.label | ... ++ | +| test.cpp:384:13:384:16 | Load: * ... | semmle.label | Load: * ... | +| test.cpp:384:14:384:16 | end | semmle.label | end | subpaths #select | test.cpp:6:14:6:15 | Load: * ... | test.cpp:4:15:4:20 | call to malloc | test.cpp:6:14:6:15 | Load: * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:4:15:4:20 | call to malloc | call to malloc | test.cpp:5:19:5:22 | size | size | @@ -1094,3 +1128,5 @@ subpaths | test.cpp:358:14:358:26 | Load: * ... | test.cpp:355:14:355:27 | new[] | test.cpp:358:14:358:26 | Load: * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@ + 1. | test.cpp:355:14:355:27 | new[] | new[] | test.cpp:356:20:356:23 | size | size | | test.cpp:359:14:359:32 | Load: * ... | test.cpp:355:14:355:27 | new[] | test.cpp:359:14:359:32 | Load: * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@ + 2. | test.cpp:355:14:355:27 | new[] | new[] | test.cpp:356:20:356:23 | size | size | | test.cpp:372:15:372:16 | Load: * ... | test.cpp:363:14:363:27 | new[] | test.cpp:372:15:372:16 | Load: * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:363:14:363:27 | new[] | new[] | test.cpp:365:19:365:22 | size | size | +| test.cpp:384:13:384:16 | Load: * ... | test.cpp:377:14:377:27 | new[] | test.cpp:384:13:384:16 | Load: * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@ + 1. | test.cpp:377:14:377:27 | new[] | new[] | test.cpp:378:20:378:23 | size | size | +| test.cpp:384:13:384:16 | Load: * ... | test.cpp:377:14:377:27 | new[] | test.cpp:384:13:384:16 | Load: * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:377:14:377:27 | new[] | new[] | test.cpp:378:20:378:23 | size | size | diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/pointer-deref/test.cpp b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/pointer-deref/test.cpp index 3711f272e76..3465affbc6e 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/pointer-deref/test.cpp +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/pointer-deref/test.cpp @@ -372,3 +372,14 @@ void test26(unsigned size) { int val = *p; // GOOD [FALSE POSITIVE] } } + +void test27(unsigned size, bool b) { + char *xs = new char[size]; + char *end = xs + size; + + if (b) { + end++; + } + + int val = *end; // BAD +} From 4a27028768a8639fe2f5f3d57b9cc5513b26faf9 Mon Sep 17 00:00:00 2001 From: Jeroen Ketema Date: Mon, 5 Jun 2023 14:55:08 +0200 Subject: [PATCH 2/3] C++: Remove `cpp/invalid-pointer-deref` results duplicating ones with smaller `k` --- .../Security/CWE/CWE-193/InvalidPointerDeref.ql | 16 ++++++++++------ .../pointer-deref/InvalidPointerDeref.expected | 1 - 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-193/InvalidPointerDeref.ql b/cpp/ql/src/experimental/Security/CWE/CWE-193/InvalidPointerDeref.ql index 610eb572d8c..20f2e934fed 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-193/InvalidPointerDeref.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-193/InvalidPointerDeref.ql @@ -377,15 +377,19 @@ predicate hasFlowPath( } from - MergedPathNode source, MergedPathNode sink, int k2, int k3, string kstr, - InvalidPointerToDerefFlow::PathNode source3, PointerArithmeticInstruction pai, string operation, - Expr offset, DataFlow::Node n + MergedPathNode source, MergedPathNode sink, int k, string kstr, PointerArithmeticInstruction pai, + string operation, Expr offset, DataFlow::Node n where - hasFlowPath(source, sink, source3, pai, operation, k3) and - invalidPointerToDerefSource(pai, source3.getNode(), k2) and + k = + min(int k2, int k3, InvalidPointerToDerefFlow::PathNode source3 | + hasFlowPath(source, sink, source3, pai, operation, k3) and + invalidPointerToDerefSource(pai, source3.getNode(), k2) + | + k2 + k3 + ) and offset = pai.getRight().getUnconvertedResultExpression() and n = source.asPathNode1().getNode() and - if (k2 + k3) = 0 then kstr = "" else kstr = " + " + (k2 + k3) + if k = 0 then kstr = "" else kstr = " + " + k select sink, source, sink, "This " + operation + " might be out of bounds, as the pointer might be equal to $@ + $@" + kstr + ".", n, n.toString(), offset, offset.toString() diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/pointer-deref/InvalidPointerDeref.expected b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/pointer-deref/InvalidPointerDeref.expected index f13d7fbf86a..056e088658b 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/pointer-deref/InvalidPointerDeref.expected +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/pointer-deref/InvalidPointerDeref.expected @@ -1128,5 +1128,4 @@ subpaths | test.cpp:358:14:358:26 | Load: * ... | test.cpp:355:14:355:27 | new[] | test.cpp:358:14:358:26 | Load: * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@ + 1. | test.cpp:355:14:355:27 | new[] | new[] | test.cpp:356:20:356:23 | size | size | | test.cpp:359:14:359:32 | Load: * ... | test.cpp:355:14:355:27 | new[] | test.cpp:359:14:359:32 | Load: * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@ + 2. | test.cpp:355:14:355:27 | new[] | new[] | test.cpp:356:20:356:23 | size | size | | test.cpp:372:15:372:16 | Load: * ... | test.cpp:363:14:363:27 | new[] | test.cpp:372:15:372:16 | Load: * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:363:14:363:27 | new[] | new[] | test.cpp:365:19:365:22 | size | size | -| test.cpp:384:13:384:16 | Load: * ... | test.cpp:377:14:377:27 | new[] | test.cpp:384:13:384:16 | Load: * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@ + 1. | test.cpp:377:14:377:27 | new[] | new[] | test.cpp:378:20:378:23 | size | size | | test.cpp:384:13:384:16 | Load: * ... | test.cpp:377:14:377:27 | new[] | test.cpp:384:13:384:16 | Load: * ... | This read might be out of bounds, as the pointer might be equal to $@ + $@. | test.cpp:377:14:377:27 | new[] | new[] | test.cpp:378:20:378:23 | size | size | From 86df424fca6400a16a4c0a4d8979a9f828e76ac4 Mon Sep 17 00:00:00 2001 From: Jeroen Ketema Date: Mon, 5 Jun 2023 15:10:54 +0200 Subject: [PATCH 3/3] C++: Fix query formatting --- .../experimental/Security/CWE/CWE-193/InvalidPointerDeref.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/src/experimental/Security/CWE/CWE-193/InvalidPointerDeref.ql b/cpp/ql/src/experimental/Security/CWE/CWE-193/InvalidPointerDeref.ql index 20f2e934fed..18c9c0f2185 100644 --- a/cpp/ql/src/experimental/Security/CWE/CWE-193/InvalidPointerDeref.ql +++ b/cpp/ql/src/experimental/Security/CWE/CWE-193/InvalidPointerDeref.ql @@ -385,7 +385,7 @@ where hasFlowPath(source, sink, source3, pai, operation, k3) and invalidPointerToDerefSource(pai, source3.getNode(), k2) | - k2 + k3 + k2 + k3 ) and offset = pai.getRight().getUnconvertedResultExpression() and n = source.asPathNode1().getNode() and