Skip to content

Commit 272ced6

Browse files
jketemajketema
authored
Merge pull request github#13374 from jketema/ptr-deref-min
C++: Remove `cpp/invalid-pointer-deref` results duplicating ones with smaller `k`
2 parents 93215ba + 86df424 commit 272ced6

File tree

3 files changed

+56
-6
lines changed

3 files changed

+56
-6
lines changed

‎cpp/ql/src/experimental/Security/CWE/CWE-193/InvalidPointerDeref.ql

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -397,15 +397,19 @@ predicate hasFlowPath(
397397
}
398398

399399
from
400-
MergedPathNode source, MergedPathNode sink, int k2, int k3, string kstr,
401-
InvalidPointerToDerefFlow::PathNode source3, PointerArithmeticInstruction pai, string operation,
402-
Expr offset, DataFlow::Node n
400+
MergedPathNode source, MergedPathNode sink, int k, string kstr, PointerArithmeticInstruction pai,
401+
string operation, Expr offset, DataFlow::Node n
403402
where
404-
hasFlowPath(source, sink, source3, pai, operation, k3) and
405-
invalidPointerToDerefSource(pai, source3.getNode(), k2) and
403+
k =
404+
min(int k2, int k3, InvalidPointerToDerefFlow::PathNode source3 |
405+
hasFlowPath(source, sink, source3, pai, operation, k3) and
406+
invalidPointerToDerefSource(pai, source3.getNode(), k2)
407+
|
408+
k2 + k3
409+
) and
406410
offset = pai.getRight().getUnconvertedResultExpression() and
407411
n = source.asPathNode1().getNode() and
408-
if (k2 + k3) = 0 then kstr = "" else kstr = " + " + (k2 + k3)
412+
if k = 0 then kstr = "" else kstr = " + " + k
409413
select sink, source, sink,
410414
"This " + operation + " might be out of bounds, as the pointer might be equal to $@ + $@" + kstr +
411415
".", n, n.toString(), offset, offset.toString()

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/pointer-deref/InvalidPointerDeref.expected

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -730,6 +730,29 @@ edges
730730
| test.cpp:368:5:368:10 | ... += ... | test.cpp:372:16:372:16 | p |
731731
| test.cpp:371:7:371:7 | p | test.cpp:372:15:372:16 | Load: * ... |
732732
| test.cpp:372:16:372:16 | p | test.cpp:372:15:372:16 | Load: * ... |
733+
| test.cpp:377:14:377:27 | new[] | test.cpp:378:15:378:16 | xs |
734+
| test.cpp:378:15:378:16 | xs | test.cpp:378:15:378:23 | ... + ... |
735+
| test.cpp:378:15:378:16 | xs | test.cpp:378:15:378:23 | ... + ... |
736+
| test.cpp:378:15:378:16 | xs | test.cpp:378:15:378:23 | ... + ... |
737+
| test.cpp:378:15:378:16 | xs | test.cpp:378:15:378:23 | ... + ... |
738+
| test.cpp:378:15:378:16 | xs | test.cpp:381:5:381:7 | end |
739+
| test.cpp:378:15:378:16 | xs | test.cpp:381:5:381:9 | ... ++ |
740+
| test.cpp:378:15:378:16 | xs | test.cpp:381:5:381:9 | ... ++ |
741+
| test.cpp:378:15:378:16 | xs | test.cpp:384:14:384:16 | end |
742+
| test.cpp:378:15:378:23 | ... + ... | test.cpp:378:15:378:23 | ... + ... |
743+
| test.cpp:378:15:378:23 | ... + ... | test.cpp:378:15:378:23 | ... + ... |
744+
| test.cpp:378:15:378:23 | ... + ... | test.cpp:381:5:381:7 | end |
745+
| test.cpp:378:15:378:23 | ... + ... | test.cpp:381:5:381:7 | end |
746+
| test.cpp:378:15:378:23 | ... + ... | test.cpp:384:13:384:16 | Load: * ... |
747+
| test.cpp:378:15:378:23 | ... + ... | test.cpp:384:13:384:16 | Load: * ... |
748+
| test.cpp:378:15:378:23 | ... + ... | test.cpp:384:13:384:16 | Load: * ... |
749+
| test.cpp:378:15:378:23 | ... + ... | test.cpp:384:13:384:16 | Load: * ... |
750+
| test.cpp:378:15:378:23 | ... + ... | test.cpp:384:14:384:16 | end |
751+
| test.cpp:378:15:378:23 | ... + ... | test.cpp:384:14:384:16 | end |
752+
| test.cpp:381:5:381:7 | end | test.cpp:384:13:384:16 | Load: * ... |
753+
| test.cpp:381:5:381:9 | ... ++ | test.cpp:384:14:384:16 | end |
754+
| test.cpp:381:5:381:9 | ... ++ | test.cpp:384:14:384:16 | end |
755+
| test.cpp:384:14:384:16 | end | test.cpp:384:13:384:16 | Load: * ... |
733756
nodes
734757
| test.cpp:4:15:4:20 | call to malloc | semmle.label | call to malloc |
735758
| test.cpp:5:15:5:15 | p | semmle.label | p |
@@ -1062,6 +1085,17 @@ nodes
10621085
| test.cpp:371:7:371:7 | p | semmle.label | p |
10631086
| test.cpp:372:15:372:16 | Load: * ... | semmle.label | Load: * ... |
10641087
| test.cpp:372:16:372:16 | p | semmle.label | p |
1088+
| test.cpp:377:14:377:27 | new[] | semmle.label | new[] |
1089+
| test.cpp:378:15:378:16 | xs | semmle.label | xs |
1090+
| test.cpp:378:15:378:23 | ... + ... | semmle.label | ... + ... |
1091+
| test.cpp:378:15:378:23 | ... + ... | semmle.label | ... + ... |
1092+
| test.cpp:378:15:378:23 | ... + ... | semmle.label | ... + ... |
1093+
| test.cpp:378:15:378:23 | ... + ... | semmle.label | ... + ... |
1094+
| test.cpp:381:5:381:7 | end | semmle.label | end |
1095+
| test.cpp:381:5:381:9 | ... ++ | semmle.label | ... ++ |
1096+
| test.cpp:381:5:381:9 | ... ++ | semmle.label | ... ++ |
1097+
| test.cpp:384:13:384:16 | Load: * ... | semmle.label | Load: * ... |
1098+
| test.cpp:384:14:384:16 | end | semmle.label | end |
10651099
subpaths
10661100
#select
10671101
| 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 |
@@ -1088,3 +1122,4 @@ subpaths
10881122
| 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 |
10891123
| 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 |
10901124
| 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 |
1125+
| 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 |

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/pointer-deref/test.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,3 +372,14 @@ void test26(unsigned size) {
372372
int val = *p; // GOOD [FALSE POSITIVE]
373373
}
374374
}
375+
376+
void test27(unsigned size, bool b) {
377+
char *xs = new char[size];
378+
char *end = xs + size;
379+
380+
if (b) {
381+
end++;
382+
}
383+
384+
int val = *end; // BAD
385+
}

0 commit comments

Comments
 (0)