Skip to content

Commit 3d037e7

Browse files
MathiasVPMathiasVP
authored
Merge pull request #16749 from MathiasVP/fix-fps-on-double-free
C++: Fix `ArrayExpr` FPs in `cpp/double-free`
2 parents b8de2ea + 4bbeda0 commit 3d037e7

File tree

6 files changed

+21
-43
lines changed

6 files changed

+21
-43
lines changed

‎cpp/ql/lib/change-notes/2024-10-06-builtin-expect.md renamed to ‎cpp/ql/lib/change-notes/2024-06-10-builtin-expect.md

File renamed without changes.

‎cpp/ql/lib/change-notes/2024-06-13-double-free.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The queries "Potential double free" (`cpp/double-free`) and "Potential use after free" (`cpp/use-after-free`) now produce fewer false positives.

‎cpp/ql/lib/semmle/code/cpp/security/flowafterfree/FlowAfterFree.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ module FlowFromFree<FlowFromFreeParamSig P> {
9595
e = any(StoreInstruction store).getDestinationAddress().getUnconvertedResultExpression()
9696
)
9797
or
98-
n.asExpr() instanceof ArrayExpr
98+
[n.asExpr(), n.asIndirectExpr()] instanceof ArrayExpr
9999
}
100100
}
101101

‎cpp/ql/test/query-tests/Critical/MemoryFreed/DoubleFree.expected

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,6 @@ edges
1313
| test_free.cpp:207:10:207:10 | pointer to free output argument | test_free.cpp:209:10:209:10 | a | provenance | |
1414
| test_free.cpp:301:12:301:14 | pointer to g_free output argument | test_free.cpp:302:12:302:14 | buf | provenance | |
1515
| test_free.cpp:319:16:319:16 | pointer to operator delete output argument | test_free.cpp:322:12:322:12 | a | provenance | |
16-
| test_free.cpp:343:12:343:24 | *access to array [post update] [ptr] | test_free.cpp:344:12:344:24 | *access to array [ptr] | provenance | |
17-
| test_free.cpp:343:12:343:24 | *access to array [post update] [ptr] | test_free.cpp:345:12:345:24 | *access to array [ptr] | provenance | |
18-
| test_free.cpp:343:12:343:24 | *access to array [post update] [ptr] | test_free.cpp:346:12:346:24 | *access to array [ptr] | provenance | |
19-
| test_free.cpp:343:26:343:28 | pointer to operator delete output argument | test_free.cpp:343:12:343:24 | *access to array [post update] [ptr] | provenance | |
20-
| test_free.cpp:344:12:344:24 | *access to array [post update] [ptr] | test_free.cpp:345:12:345:24 | *access to array [ptr] | provenance | |
21-
| test_free.cpp:344:12:344:24 | *access to array [post update] [ptr] | test_free.cpp:346:12:346:24 | *access to array [ptr] | provenance | |
22-
| test_free.cpp:344:12:344:24 | *access to array [ptr] | test_free.cpp:344:26:344:28 | ptr | provenance | |
23-
| test_free.cpp:344:26:344:28 | pointer to operator delete output argument | test_free.cpp:344:12:344:24 | *access to array [post update] [ptr] | provenance | |
24-
| test_free.cpp:345:12:345:24 | *access to array [post update] [ptr] | test_free.cpp:346:12:346:24 | *access to array [ptr] | provenance | |
25-
| test_free.cpp:345:12:345:24 | *access to array [ptr] | test_free.cpp:345:26:345:28 | ptr | provenance | |
26-
| test_free.cpp:345:12:345:24 | *access to array [ptr] | test_free.cpp:345:26:345:28 | ptr | provenance | |
27-
| test_free.cpp:345:26:345:28 | pointer to operator delete output argument | test_free.cpp:345:12:345:24 | *access to array [post update] [ptr] | provenance | |
28-
| test_free.cpp:346:12:346:24 | *access to array [ptr] | test_free.cpp:346:26:346:28 | ptr | provenance | |
29-
| test_free.cpp:346:12:346:24 | *access to array [ptr] | test_free.cpp:346:26:346:28 | ptr | provenance | |
30-
| test_free.cpp:346:12:346:24 | *access to array [ptr] | test_free.cpp:346:26:346:28 | ptr | provenance | |
3116
nodes
3217
| test_free.cpp:11:10:11:10 | pointer to free output argument | semmle.label | pointer to free output argument |
3318
| test_free.cpp:14:10:14:10 | a | semmle.label | a |
@@ -57,24 +42,6 @@ nodes
5742
| test_free.cpp:302:12:302:14 | buf | semmle.label | buf |
5843
| test_free.cpp:319:16:319:16 | pointer to operator delete output argument | semmle.label | pointer to operator delete output argument |
5944
| test_free.cpp:322:12:322:12 | a | semmle.label | a |
60-
| test_free.cpp:343:12:343:24 | *access to array [post update] [ptr] | semmle.label | *access to array [post update] [ptr] |
61-
| test_free.cpp:343:26:343:28 | pointer to operator delete output argument | semmle.label | pointer to operator delete output argument |
62-
| test_free.cpp:344:12:344:24 | *access to array [post update] [ptr] | semmle.label | *access to array [post update] [ptr] |
63-
| test_free.cpp:344:12:344:24 | *access to array [ptr] | semmle.label | *access to array [ptr] |
64-
| test_free.cpp:344:26:344:28 | pointer to operator delete output argument | semmle.label | pointer to operator delete output argument |
65-
| test_free.cpp:344:26:344:28 | ptr | semmle.label | ptr |
66-
| test_free.cpp:345:12:345:24 | *access to array [post update] [ptr] | semmle.label | *access to array [post update] [ptr] |
67-
| test_free.cpp:345:12:345:24 | *access to array [ptr] | semmle.label | *access to array [ptr] |
68-
| test_free.cpp:345:12:345:24 | *access to array [ptr] | semmle.label | *access to array [ptr] |
69-
| test_free.cpp:345:26:345:28 | pointer to operator delete output argument | semmle.label | pointer to operator delete output argument |
70-
| test_free.cpp:345:26:345:28 | ptr | semmle.label | ptr |
71-
| test_free.cpp:345:26:345:28 | ptr | semmle.label | ptr |
72-
| test_free.cpp:346:12:346:24 | *access to array [ptr] | semmle.label | *access to array [ptr] |
73-
| test_free.cpp:346:12:346:24 | *access to array [ptr] | semmle.label | *access to array [ptr] |
74-
| test_free.cpp:346:12:346:24 | *access to array [ptr] | semmle.label | *access to array [ptr] |
75-
| test_free.cpp:346:26:346:28 | ptr | semmle.label | ptr |
76-
| test_free.cpp:346:26:346:28 | ptr | semmle.label | ptr |
77-
| test_free.cpp:346:26:346:28 | ptr | semmle.label | ptr |
7845
subpaths
7946
#select
8047
| test_free.cpp:14:10:14:10 | a | test_free.cpp:11:10:11:10 | pointer to free output argument | test_free.cpp:14:10:14:10 | a | Memory pointed to by $@ may already have been freed by $@. | test_free.cpp:14:10:14:10 | a | a | test_free.cpp:11:5:11:8 | call to free | call to free |
@@ -91,9 +58,3 @@ subpaths
9158
| test_free.cpp:209:10:209:10 | a | test_free.cpp:207:10:207:10 | pointer to free output argument | test_free.cpp:209:10:209:10 | a | Memory pointed to by $@ may already have been freed by $@. | test_free.cpp:209:10:209:10 | a | a | test_free.cpp:207:5:207:8 | call to free | call to free |
9259
| test_free.cpp:302:12:302:14 | buf | test_free.cpp:301:12:301:14 | pointer to g_free output argument | test_free.cpp:302:12:302:14 | buf | Memory pointed to by $@ may already have been freed by $@. | test_free.cpp:302:12:302:14 | buf | buf | test_free.cpp:301:5:301:10 | call to g_free | call to g_free |
9360
| test_free.cpp:322:12:322:12 | a | test_free.cpp:319:16:319:16 | pointer to operator delete output argument | test_free.cpp:322:12:322:12 | a | Memory pointed to by $@ may already have been freed by $@. | test_free.cpp:322:12:322:12 | a | a | test_free.cpp:319:9:319:16 | delete | delete |
94-
| test_free.cpp:344:26:344:28 | ptr | test_free.cpp:343:26:343:28 | pointer to operator delete output argument | test_free.cpp:344:26:344:28 | ptr | Memory pointed to by $@ may already have been freed by $@. | test_free.cpp:344:26:344:28 | ptr | ptr | test_free.cpp:343:5:343:28 | delete | delete |
95-
| test_free.cpp:345:26:345:28 | ptr | test_free.cpp:343:26:343:28 | pointer to operator delete output argument | test_free.cpp:345:26:345:28 | ptr | Memory pointed to by $@ may already have been freed by $@. | test_free.cpp:345:26:345:28 | ptr | ptr | test_free.cpp:343:5:343:28 | delete | delete |
96-
| test_free.cpp:345:26:345:28 | ptr | test_free.cpp:344:26:344:28 | pointer to operator delete output argument | test_free.cpp:345:26:345:28 | ptr | Memory pointed to by $@ may already have been freed by $@. | test_free.cpp:345:26:345:28 | ptr | ptr | test_free.cpp:344:5:344:28 | delete | delete |
97-
| test_free.cpp:346:26:346:28 | ptr | test_free.cpp:343:26:343:28 | pointer to operator delete output argument | test_free.cpp:346:26:346:28 | ptr | Memory pointed to by $@ may already have been freed by $@. | test_free.cpp:346:26:346:28 | ptr | ptr | test_free.cpp:343:5:343:28 | delete | delete |
98-
| test_free.cpp:346:26:346:28 | ptr | test_free.cpp:344:26:344:28 | pointer to operator delete output argument | test_free.cpp:346:26:346:28 | ptr | Memory pointed to by $@ may already have been freed by $@. | test_free.cpp:346:26:346:28 | ptr | ptr | test_free.cpp:344:5:344:28 | delete | delete |
99-
| test_free.cpp:346:26:346:28 | ptr | test_free.cpp:345:26:345:28 | pointer to operator delete output argument | test_free.cpp:346:26:346:28 | ptr | Memory pointed to by $@ may already have been freed by $@. | test_free.cpp:346:26:346:28 | ptr | ptr | test_free.cpp:345:5:345:28 | delete | delete |

‎cpp/ql/test/query-tests/Critical/MemoryFreed/MemoryFreed.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,8 @@
115115
| test_free.cpp:344:26:344:28 | ptr |
116116
| test_free.cpp:345:26:345:28 | ptr |
117117
| test_free.cpp:346:26:346:28 | ptr |
118+
| test_free.cpp:356:19:356:19 | a |
119+
| test_free.cpp:357:19:357:19 | a |
118120
| virtual.cpp:18:10:18:10 | a |
119121
| virtual.cpp:19:10:19:10 | c |
120122
| virtual.cpp:38:10:38:10 | b |

‎cpp/ql/test/query-tests/Critical/MemoryFreed/test_free.cpp

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,18 @@ struct PtrContainer {
341341

342342
void test_array(PtrContainer *containers) {
343343
delete containers[0].ptr; // GOOD
344-
delete containers[1].ptr; // GOOD [FALSE POSITIVE]
345-
delete containers[2].ptr; // GOOD [FALSE POSITIVE]
346-
delete containers[2].ptr; // BAD (double free)
344+
delete containers[1].ptr; // GOOD
345+
delete containers[2].ptr; // GOOD
346+
delete containers[2].ptr; // BAD (double free) [NOT DETECTED]
347347
}
348+
349+
struct E {
350+
struct EC {
351+
int* a;
352+
} ec[2];
353+
};
354+
355+
void test(E* e) {
356+
free(e->ec[0].a);
357+
free(e->ec[1].a); // GOOD
358+
}

0 commit comments

Comments
 (0)