Skip to content

Commit f3914ff

Browse files
authored
Merge pull request github#11823 from geoffw0/heuristicalloc
C++: Use HeuristicAllocationExpr in more queries
2 parents f5e5f6d + bb451f3 commit f3914ff

File tree

18 files changed

+125
-16
lines changed

18 files changed

+125
-16
lines changed

cpp/ql/lib/semmle/code/cpp/models/interfaces/Allocation.qll

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,13 +133,15 @@ abstract class HeuristicAllocationExpr extends Expr {
133133

134134
/**
135135
* Gets a constant multiplier for the allocation size given by `getSizeExpr`,
136-
* in bytes.
136+
* in bytes. This predicate should be used with caution as it can be
137+
* inaccurate for allocations identified using heuristics.
137138
*/
138139
int getSizeMult() { none() }
139140

140141
/**
141142
* Gets the size of this allocation in bytes, if it is a fixed size and that
142-
* size can be determined.
143+
* size can be determined. This predicate should be used with caution as it
144+
* can be inaccurate for allocations identified using heuristics.
143145
*/
144146
int getSizeBytes() { none() }
145147

cpp/ql/src/Security/CWE/CWE-131/NoSpaceForZeroTerminator.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import semmle.code.cpp.models.interfaces.ArrayFunction
2121
import semmle.code.cpp.models.interfaces.Allocation
2222
import semmle.code.cpp.commons.NullTermination
2323

24-
predicate terminationProblem(AllocationExpr malloc, string msg) {
24+
predicate terminationProblem(HeuristicAllocationExpr malloc, string msg) {
2525
// malloc(strlen(...))
2626
exists(StrlenCall strlen | DataFlow::localExprFlow(strlen, malloc.getSizeExpr())) and
2727
// flows to a call that implies this is a null-terminated string

cpp/ql/src/Security/CWE/CWE-190/TaintedAllocationSize.ql

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,8 @@ import DataFlow::PathGraph
2525
* Holds if `alloc` is an allocation, and `tainted` is a child of it that is a
2626
* taint sink.
2727
*/
28-
predicate allocSink(Expr alloc, DataFlow::Node sink) {
28+
predicate allocSink(HeuristicAllocationExpr alloc, DataFlow::Node sink) {
2929
exists(Expr e | e = sink.asConvertedExpr() |
30-
isAllocationExpr(alloc) and
3130
e = alloc.getAChild() and
3231
e.getUnspecifiedType() instanceof IntegralType
3332
)
@@ -89,6 +88,10 @@ class TaintedAllocationSizeConfiguration extends TaintTracking::Configuration {
8988
readsVariable(access.getDef(), checkedVar) and
9089
nodeIsBarrierEqualityCandidate(node, access, checkedVar)
9190
)
91+
or
92+
// block flow to inside of identified allocation functions (this flow leads
93+
// to duplicate results)
94+
any(HeuristicAllocationFunction f).getAParameter() = node.asParameter()
9295
}
9396
}
9497

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 `cpp/no-space-for-terminator` and `cpp/uncontrolled-allocation-size` queries have been enhanced with heuristic detection of allocations. These queries now find more results.

cpp/ql/src/experimental/Security/CWE/CWE-190/AllocMultiplicationOverflow.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class MultToAllocConfig extends DataFlow::Configuration {
2929

3030
override predicate isSink(DataFlow::Node node) {
3131
// something that affects an allocation size
32-
node.asExpr() = any(AllocationExpr ae).getSizeExpr().getAChild*()
32+
node.asExpr() = any(HeuristicAllocationExpr ae).getSizeExpr().getAChild*()
3333
}
3434
}
3535

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-190/AllocMultiplicationOverflow/AllocMultiplicationOverflow.expected

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
edges
22
| test.cpp:22:17:22:21 | ... * ... | test.cpp:23:33:23:37 | size1 |
3+
| test.cpp:37:24:37:27 | size | test.cpp:37:46:37:49 | size |
4+
| test.cpp:45:36:45:40 | ... * ... | test.cpp:37:24:37:27 | size |
35
nodes
46
| test.cpp:13:33:13:37 | ... * ... | semmle.label | ... * ... |
57
| test.cpp:15:31:15:35 | ... * ... | semmle.label | ... * ... |
@@ -8,6 +10,11 @@ nodes
810
| test.cpp:23:33:23:37 | size1 | semmle.label | size1 |
911
| test.cpp:30:27:30:31 | ... * ... | semmle.label | ... * ... |
1012
| test.cpp:31:27:31:31 | ... * ... | semmle.label | ... * ... |
13+
| test.cpp:37:24:37:27 | size | semmle.label | size |
14+
| test.cpp:37:46:37:49 | size | semmle.label | size |
15+
| test.cpp:45:36:45:40 | ... * ... | semmle.label | ... * ... |
16+
| test.cpp:45:36:45:40 | ... * ... | semmle.label | ... * ... |
17+
| test.cpp:46:36:46:40 | ... * ... | semmle.label | ... * ... |
1118
subpaths
1219
#select
1320
| test.cpp:13:33:13:37 | ... * ... | test.cpp:13:33:13:37 | ... * ... | test.cpp:13:33:13:37 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:13:33:13:37 | ... * ... | multiplication |
@@ -16,3 +23,6 @@ subpaths
1623
| test.cpp:23:33:23:37 | size1 | test.cpp:22:17:22:21 | ... * ... | test.cpp:23:33:23:37 | size1 | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:22:17:22:21 | ... * ... | multiplication |
1724
| test.cpp:30:27:30:31 | ... * ... | test.cpp:30:27:30:31 | ... * ... | test.cpp:30:27:30:31 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:30:27:30:31 | ... * ... | multiplication |
1825
| test.cpp:31:27:31:31 | ... * ... | test.cpp:31:27:31:31 | ... * ... | test.cpp:31:27:31:31 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:31:27:31:31 | ... * ... | multiplication |
26+
| test.cpp:37:46:37:49 | size | test.cpp:45:36:45:40 | ... * ... | test.cpp:37:46:37:49 | size | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:45:36:45:40 | ... * ... | multiplication |
27+
| test.cpp:45:36:45:40 | ... * ... | test.cpp:45:36:45:40 | ... * ... | test.cpp:45:36:45:40 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:45:36:45:40 | ... * ... | multiplication |
28+
| test.cpp:46:36:46:40 | ... * ... | test.cpp:46:36:46:40 | ... * ... | test.cpp:46:36:46:40 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:46:36:46:40 | ... * ... | multiplication |

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-190/AllocMultiplicationOverflow/test.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,3 +30,18 @@ void test()
3030
char *buffer8 = new char[x * y]; // BAD
3131
char *buffer9 = new char[x * x]; // BAD
3232
}
33+
34+
35+
// --- custom allocators ---
36+
37+
void *MyMalloc1(size_t size) { return malloc(size); } // [additional detection here]
38+
void *MyMalloc2(size_t size);
39+
40+
void customAllocatorTests()
41+
{
42+
int x = getAnInt();
43+
int y = getAnInt();
44+
45+
char *buffer1 = (char *)MyMalloc1(x * y); // BAD
46+
char *buffer2 = (char *)MyMalloc2(x * y); // BAD
47+
}

cpp/ql/test/query-tests/Critical/OverflowCalculated/NoSpaceForZeroTerminator.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,5 @@
77
| tests3.cpp:25:21:25:31 | call to malloc | This allocation does not include space to null-terminate the string. |
88
| tests3.cpp:30:21:30:31 | call to malloc | This allocation does not include space to null-terminate the string. |
99
| tests3.cpp:53:17:53:44 | new[] | This allocation does not include space to null-terminate the string. |
10+
| tests3.cpp:81:20:81:28 | call to MyMalloc1 | This allocation does not include space to null-terminate the string. |
11+
| tests3.cpp:84:20:84:28 | call to MyMalloc2 | This allocation does not include space to null-terminate the string. |

cpp/ql/test/query-tests/Critical/OverflowCalculated/tests3.cpp

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// tests1.cpp
1+
// tests3.cpp
22

33
typedef unsigned int size_t;
44

@@ -66,3 +66,21 @@ void test3c()
6666

6767
delete buffer;
6868
}
69+
70+
// --- custom allocators ---
71+
72+
void *MyMalloc1(size_t size) { return std::malloc(size); }
73+
void *MyMalloc2(size_t size);
74+
75+
void tests4()
76+
{
77+
const char *str4 = "1234";
78+
char *buffer1 = 0;
79+
char *buffer2 = 0;
80+
81+
buffer1 = (char *)MyMalloc1(strlen(str4)); // BAD
82+
strcpy(buffer1, str4);
83+
84+
buffer2 = (char *)MyMalloc2(strlen(str4)); // BAD
85+
strcpy(buffer2, str4);
86+
}

cpp/ql/test/query-tests/Critical/SizeCheck/test.c

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,3 +58,14 @@ void test_union() {
5858
MyUnion *a = malloc(sizeof(MyUnion)); // GOOD
5959
MyUnion *b = malloc(sizeof(MyStruct)); // BAD (too small)
6060
}
61+
62+
// --- custom allocators ---
63+
64+
void *MyMalloc1(size_t size) { return malloc(size); }
65+
void *MyMalloc2(size_t size);
66+
67+
void customAllocatorTests()
68+
{
69+
float *fptr1 = MyMalloc1(3); // BAD (too small) [NOT DETECTED]
70+
float *fptr2 = MyMalloc2(3); // BAD (too small) [NOT DETECTED]
71+
}

0 commit comments

Comments
 (0)