Skip to content

Commit 04f4039

Browse files
authored
Merge pull request github#17354 from paldepind/realloc-data-flow
C++: Make realloc a data-flow function
2 parents 99400fe + 7564304 commit 04f4039

File tree

11 files changed

+95
-66
lines changed

11 files changed

+95
-66
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added a data flow model for `realloc`-like functions, which were previously modeled as a taint tracking functions. This change improves the precision of queries where flow through `realloc`-like functions might affect the results.

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@
55
*/
66

77
import semmle.code.cpp.models.interfaces.Allocation
8-
import semmle.code.cpp.models.interfaces.Taint
8+
import semmle.code.cpp.models.interfaces.DataFlow
99

1010
/**
1111
* An allocation function (such as `realloc`) that has an argument for the size
1212
* in bytes, and an argument for an existing pointer that is to be reallocated.
1313
*/
14-
private class ReallocAllocationFunction extends AllocationFunction, TaintFunction {
14+
private class ReallocAllocationFunction extends AllocationFunction, DataFlowFunction {
1515
int sizeArg;
1616
int reallocArg;
1717

@@ -44,7 +44,7 @@ private class ReallocAllocationFunction extends AllocationFunction, TaintFunctio
4444

4545
override int getReallocPtrArg() { result = reallocArg }
4646

47-
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
47+
override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
4848
input.isParameterDeref(this.getReallocPtrArg()) and output.isReturnValueDeref()
4949
}
5050
}

cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ argHasPostUpdate
3333
| test.cpp:67:29:67:35 | source1 | ArgumentNode is missing PostUpdateNode. |
3434
| test.cpp:813:19:813:35 | * ... | ArgumentNode is missing PostUpdateNode. |
3535
| test.cpp:848:23:848:25 | rpx | ArgumentNode is missing PostUpdateNode. |
36-
| test.cpp:1057:19:1057:21 | * ... | ArgumentNode is missing PostUpdateNode. |
36+
| test.cpp:1065:19:1065:21 | * ... | ArgumentNode is missing PostUpdateNode. |
3737
postWithInFlow
3838
| BarrierGuard.cpp:49:6:49:6 | x [post update] | PostUpdateNode should not be the target of local flow. |
3939
| BarrierGuard.cpp:60:7:60:7 | x [post update] | PostUpdateNode should not be the target of local flow. |
@@ -167,15 +167,15 @@ postWithInFlow
167167
| test.cpp:932:5:932:19 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
168168
| test.cpp:932:6:932:19 | global_pointer [inner post update] | PostUpdateNode should not be the target of local flow. |
169169
| test.cpp:1045:9:1045:11 | ref arg buf | PostUpdateNode should not be the target of local flow. |
170-
| test.cpp:1051:5:1051:11 | content [post update] | PostUpdateNode should not be the target of local flow. |
171-
| test.cpp:1052:9:1052:9 | a [inner post update] | PostUpdateNode should not be the target of local flow. |
172-
| test.cpp:1056:5:1056:7 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
173-
| test.cpp:1056:6:1056:7 | pp [inner post update] | PostUpdateNode should not be the target of local flow. |
174-
| test.cpp:1062:53:1062:53 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
175-
| test.cpp:1072:3:1072:4 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
176-
| test.cpp:1072:4:1072:4 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
177-
| test.cpp:1073:3:1073:4 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
178-
| test.cpp:1073:4:1073:4 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
170+
| test.cpp:1059:5:1059:11 | content [post update] | PostUpdateNode should not be the target of local flow. |
171+
| test.cpp:1060:9:1060:9 | a [inner post update] | PostUpdateNode should not be the target of local flow. |
172+
| test.cpp:1064:5:1064:7 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
173+
| test.cpp:1064:6:1064:7 | pp [inner post update] | PostUpdateNode should not be the target of local flow. |
174+
| test.cpp:1070:53:1070:53 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
175+
| test.cpp:1080:3:1080:4 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
176+
| test.cpp:1080:4:1080:4 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
177+
| test.cpp:1081:3:1081:4 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
178+
| test.cpp:1081:4:1081:4 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
179179
viableImplInCallContextTooLarge
180180
uniqueParameterNodeAtPosition
181181
uniqueParameterNodePosition

cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow-ir.expected

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -202,12 +202,12 @@
202202
| test.cpp:489:23:489:29 | *content | test.cpp:490:8:490:17 | * ... |
203203
| test.cpp:489:23:489:29 | content | test.cpp:489:23:489:29 | content |
204204
| test.cpp:489:23:489:29 | content | test.cpp:490:9:490:17 | p_content |
205-
| test.cpp:1050:12:1050:12 | definition of a | test.cpp:1051:3:1051:3 | *a |
206-
| test.cpp:1051:3:1051:3 | *a | test.cpp:1052:8:1052:9 | *& ... |
207-
| test.cpp:1051:3:1051:3 | *a [post update] | test.cpp:1052:8:1052:9 | *& ... |
208-
| test.cpp:1051:3:1051:3 | a | test.cpp:1052:8:1052:9 | & ... |
209-
| test.cpp:1051:3:1051:3 | a [post update] | test.cpp:1052:8:1052:9 | & ... |
210-
| test.cpp:1051:15:1051:21 | 0 | test.cpp:1051:3:1051:21 | ... = ... |
211-
| test.cpp:1051:15:1051:21 | *0 | test.cpp:1051:3:1051:21 | *... = ... |
212-
| test.cpp:1052:9:1052:9 | *a | test.cpp:1052:8:1052:9 | *& ... |
213-
| test.cpp:1052:9:1052:9 | a | test.cpp:1052:8:1052:9 | & ... |
205+
| test.cpp:1058:12:1058:12 | definition of a | test.cpp:1059:3:1059:3 | *a |
206+
| test.cpp:1059:3:1059:3 | *a | test.cpp:1060:8:1060:9 | *& ... |
207+
| test.cpp:1059:3:1059:3 | *a [post update] | test.cpp:1060:8:1060:9 | *& ... |
208+
| test.cpp:1059:3:1059:3 | a | test.cpp:1060:8:1060:9 | & ... |
209+
| test.cpp:1059:3:1059:3 | a [post update] | test.cpp:1060:8:1060:9 | & ... |
210+
| test.cpp:1059:15:1059:21 | 0 | test.cpp:1059:3:1059:21 | ... = ... |
211+
| test.cpp:1059:15:1059:21 | *0 | test.cpp:1059:3:1059:21 | *... = ... |
212+
| test.cpp:1060:9:1060:9 | *a | test.cpp:1060:8:1060:9 | *& ... |
213+
| test.cpp:1060:9:1060:9 | a | test.cpp:1060:8:1060:9 | & ... |

cpp/ql/test/library-tests/dataflow/dataflow-tests/localFlow.expected

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,10 @@ WARNING: module 'DataFlow' has been deprecated and may be removed in future (loc
8181
| test.cpp:488:21:488:21 | s [post update] | test.cpp:489:20:489:20 | s |
8282
| test.cpp:488:24:488:30 | ref arg content | test.cpp:489:23:489:29 | content |
8383
| test.cpp:489:23:489:29 | content | test.cpp:490:9:490:17 | p_content |
84-
| test.cpp:1050:12:1050:12 | a | test.cpp:1051:3:1051:3 | a |
85-
| test.cpp:1050:12:1050:12 | a | test.cpp:1052:9:1052:9 | a |
86-
| test.cpp:1051:3:1051:3 | a [post update] | test.cpp:1052:9:1052:9 | a |
87-
| test.cpp:1051:3:1051:21 | ... = ... | test.cpp:1051:5:1051:11 | content [post update] |
88-
| test.cpp:1051:15:1051:21 | 0 | test.cpp:1051:3:1051:21 | ... = ... |
89-
| test.cpp:1052:8:1052:9 | ref arg & ... | test.cpp:1052:9:1052:9 | a [inner post update] |
90-
| test.cpp:1052:9:1052:9 | a | test.cpp:1052:8:1052:9 | & ... |
84+
| test.cpp:1058:12:1058:12 | a | test.cpp:1059:3:1059:3 | a |
85+
| test.cpp:1058:12:1058:12 | a | test.cpp:1060:9:1060:9 | a |
86+
| test.cpp:1059:3:1059:3 | a [post update] | test.cpp:1060:9:1060:9 | a |
87+
| test.cpp:1059:3:1059:21 | ... = ... | test.cpp:1059:5:1059:11 | content [post update] |
88+
| test.cpp:1059:15:1059:21 | 0 | test.cpp:1059:3:1059:21 | ... = ... |
89+
| test.cpp:1060:8:1060:9 | ref arg & ... | test.cpp:1060:9:1060:9 | a [inner post update] |
90+
| test.cpp:1060:9:1060:9 | a | test.cpp:1060:8:1060:9 | & ... |

cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ astFlow
127127
| test.cpp:842:11:842:16 | call to source | test.cpp:844:8:844:8 | y |
128128
| test.cpp:846:13:846:27 | call to indirect_source | test.cpp:848:23:848:25 | rpx |
129129
| test.cpp:860:54:860:59 | call to source | test.cpp:861:10:861:37 | static_local_pointer_dynamic |
130-
| test.cpp:1050:12:1050:12 | a | test.cpp:1052:8:1052:9 | & ... |
130+
| test.cpp:1058:12:1058:12 | a | test.cpp:1060:8:1060:9 | & ... |
131131
| true_upon_entry.cpp:17:11:17:16 | call to source | true_upon_entry.cpp:21:8:21:8 | x |
132132
| true_upon_entry.cpp:27:9:27:14 | call to source | true_upon_entry.cpp:29:8:29:8 | x |
133133
| true_upon_entry.cpp:33:11:33:16 | call to source | true_upon_entry.cpp:39:8:39:8 | x |
@@ -313,7 +313,8 @@ irFlow
313313
| test.cpp:1021:18:1021:32 | *call to indirect_source | test.cpp:1027:19:1027:28 | *translated |
314314
| test.cpp:1021:18:1021:32 | *call to indirect_source | test.cpp:1031:19:1031:28 | *translated |
315315
| test.cpp:1045:14:1045:19 | call to source | test.cpp:1046:7:1046:10 | * ... |
316-
| test.cpp:1081:27:1081:34 | call to source | test.cpp:1081:27:1081:34 | call to source |
316+
| test.cpp:1052:13:1052:27 | *call to indirect_source | test.cpp:1054:7:1054:11 | * ... |
317+
| test.cpp:1089:27:1089:34 | call to source | test.cpp:1089:27:1089:34 | call to source |
317318
| true_upon_entry.cpp:9:11:9:16 | call to source | true_upon_entry.cpp:13:8:13:8 | x |
318319
| true_upon_entry.cpp:17:11:17:16 | call to source | true_upon_entry.cpp:21:8:21:8 | x |
319320
| true_upon_entry.cpp:27:9:27:14 | call to source | true_upon_entry.cpp:29:8:29:8 | x |

cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,6 +1046,14 @@ void memset_test(char* buf) { // $ ast-def=buf ir-def=*buf
10461046
sink(*buf); // $ ir MISSING: ast
10471047
}
10481048

1049+
void *realloc(void *, size_t);
1050+
1051+
void test_realloc() {
1052+
int *src = indirect_source();
1053+
int *dest = (int*)realloc(src, sizeof(int));
1054+
sink(*dest); // $ ir, MISSING: ast
1055+
}
1056+
10491057
void flow_out_of_address_with_local_flow() {
10501058
MyStruct a;
10511059
a.content = nullptr;

cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.expected

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,5 +51,5 @@ incorrectBaseType
5151
| test.cpp:848:23:848:25 | rpx | Expected 'Node.getType()' to be int, but it was int * |
5252
| test.cpp:854:10:854:36 | * ... | Expected 'Node.getType()' to be const int, but it was int |
5353
| test.cpp:867:10:867:30 | * ... | Expected 'Node.getType()' to be const int, but it was int |
54-
| test.cpp:1062:52:1062:53 | *& ... | Expected 'Node.getType()' to be char, but it was char * |
54+
| test.cpp:1070:52:1070:53 | *& ... | Expected 'Node.getType()' to be char, but it was char * |
5555
failures

cpp/ql/test/library-tests/dataflow/dataflow-tests/uninitialized.expected

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,5 +54,5 @@
5454
| test.cpp:796:12:796:12 | a | test.cpp:797:20:797:20 | a |
5555
| test.cpp:796:12:796:12 | a | test.cpp:797:31:797:31 | a |
5656
| test.cpp:796:12:796:12 | a | test.cpp:798:17:798:17 | a |
57-
| test.cpp:1050:12:1050:12 | a | test.cpp:1051:3:1051:3 | a |
58-
| test.cpp:1050:12:1050:12 | a | test.cpp:1052:9:1052:9 | a |
57+
| test.cpp:1058:12:1058:12 | a | test.cpp:1059:3:1059:3 | a |
58+
| test.cpp:1058:12:1058:12 | a | test.cpp:1060:9:1060:9 | a |

cpp/ql/test/library-tests/dataflow/taint-tests/localTaint.expected

Lines changed: 39 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -6597,38 +6597,45 @@ WARNING: module 'TaintTracking' has been deprecated and may be removed in future
65976597
| taint.cpp:729:27:729:32 | endptr | taint.cpp:729:26:729:32 | & ... | |
65986598
| taint.cpp:731:7:731:12 | ref arg endptr | taint.cpp:732:8:732:13 | endptr | |
65996599
| taint.cpp:732:8:732:13 | endptr | taint.cpp:732:7:732:13 | * ... | TAINT |
6600-
| taint.cpp:738:17:738:31 | call to indirect_source | taint.cpp:739:30:739:35 | source | |
6601-
| taint.cpp:739:22:739:28 | call to realloc | taint.cpp:740:7:740:10 | dest | |
6602-
| taint.cpp:739:30:739:35 | source | taint.cpp:739:22:739:28 | call to realloc | TAINT |
6603-
| taint.cpp:743:40:743:45 | buffer | taint.cpp:744:5:744:10 | buffer | |
6604-
| taint.cpp:743:40:743:45 | buffer | taint.cpp:745:27:745:32 | buffer | |
6605-
| taint.cpp:744:4:744:10 | * ... | taint.cpp:744:3:744:10 | * ... | TAINT |
6606-
| taint.cpp:744:5:744:10 | buffer | taint.cpp:744:4:744:10 | * ... | TAINT |
6607-
| taint.cpp:744:14:744:19 | call to source | taint.cpp:744:3:744:21 | ... = ... | |
6608-
| taint.cpp:745:19:745:25 | call to realloc | taint.cpp:743:40:743:45 | buffer | |
6609-
| taint.cpp:745:19:745:25 | call to realloc | taint.cpp:745:3:745:37 | ... = ... | |
6610-
| taint.cpp:745:19:745:25 | call to realloc | taint.cpp:746:10:746:15 | buffer | |
6611-
| taint.cpp:745:27:745:32 | buffer | taint.cpp:745:19:745:25 | call to realloc | TAINT |
6612-
| taint.cpp:746:9:746:15 | * ... | taint.cpp:746:8:746:15 | * ... | TAINT |
6613-
| taint.cpp:746:10:746:15 | buffer | taint.cpp:746:9:746:15 | * ... | TAINT |
6614-
| taint.cpp:751:31:751:34 | path | taint.cpp:751:31:751:34 | path | |
6615-
| taint.cpp:751:31:751:34 | path | taint.cpp:752:10:752:13 | path | |
6616-
| taint.cpp:751:31:751:34 | path | taint.cpp:753:10:753:13 | path | |
6617-
| taint.cpp:751:43:751:46 | data | taint.cpp:751:43:751:46 | data | |
6618-
| taint.cpp:751:43:751:46 | data | taint.cpp:753:22:753:25 | data | |
6619-
| taint.cpp:752:10:752:13 | ref arg path | taint.cpp:751:31:751:34 | path | |
6620-
| taint.cpp:752:10:752:13 | ref arg path | taint.cpp:753:10:753:13 | path | |
6621-
| taint.cpp:752:16:752:19 | %s | taint.cpp:752:10:752:13 | ref arg path | TAINT |
6622-
| taint.cpp:752:22:752:26 | abc | taint.cpp:752:10:752:13 | ref arg path | TAINT |
6623-
| taint.cpp:753:10:753:13 | ref arg path | taint.cpp:751:31:751:34 | path | |
6624-
| taint.cpp:753:16:753:19 | %s | taint.cpp:753:10:753:13 | ref arg path | TAINT |
6625-
| taint.cpp:753:22:753:25 | data | taint.cpp:753:10:753:13 | ref arg path | TAINT |
6626-
| taint.cpp:753:22:753:25 | ref arg data | taint.cpp:751:43:751:46 | data | |
6627-
| taint.cpp:757:7:757:10 | path | taint.cpp:758:21:758:24 | path | |
6628-
| taint.cpp:757:7:757:10 | path | taint.cpp:759:8:759:11 | path | |
6629-
| taint.cpp:758:21:758:24 | ref arg path | taint.cpp:759:8:759:11 | path | |
6630-
| taint.cpp:759:8:759:11 | path | taint.cpp:759:7:759:11 | * ... | |
6631-
| taint.cpp:769:37:769:42 | call to source | taint.cpp:770:7:770:9 | obj | |
6600+
| taint.cpp:739:17:739:31 | call to indirect_source | taint.cpp:740:30:740:35 | source | |
6601+
| taint.cpp:740:22:740:28 | call to realloc | taint.cpp:741:7:741:10 | dest | |
6602+
| taint.cpp:740:30:740:35 | source | taint.cpp:740:22:740:28 | call to realloc | TAINT |
6603+
| taint.cpp:744:40:744:45 | buffer | taint.cpp:745:5:745:10 | buffer | |
6604+
| taint.cpp:744:40:744:45 | buffer | taint.cpp:746:27:746:32 | buffer | |
6605+
| taint.cpp:745:4:745:10 | * ... | taint.cpp:745:3:745:10 | * ... | TAINT |
6606+
| taint.cpp:745:5:745:10 | buffer | taint.cpp:745:4:745:10 | * ... | TAINT |
6607+
| taint.cpp:745:14:745:19 | call to source | taint.cpp:745:3:745:21 | ... = ... | |
6608+
| taint.cpp:746:19:746:25 | call to realloc | taint.cpp:744:40:744:45 | buffer | |
6609+
| taint.cpp:746:19:746:25 | call to realloc | taint.cpp:746:3:746:37 | ... = ... | |
6610+
| taint.cpp:746:19:746:25 | call to realloc | taint.cpp:747:10:747:15 | buffer | |
6611+
| taint.cpp:746:27:746:32 | buffer | taint.cpp:746:19:746:25 | call to realloc | TAINT |
6612+
| taint.cpp:747:9:747:15 | * ... | taint.cpp:747:8:747:15 | * ... | TAINT |
6613+
| taint.cpp:747:10:747:15 | buffer | taint.cpp:747:9:747:15 | * ... | TAINT |
6614+
| taint.cpp:752:13:752:18 | call to malloc | taint.cpp:753:2:753:2 | a | |
6615+
| taint.cpp:752:13:752:18 | call to malloc | taint.cpp:754:22:754:22 | a | |
6616+
| taint.cpp:753:2:753:2 | a [post update] | taint.cpp:754:22:754:22 | a | |
6617+
| taint.cpp:753:2:753:16 | ... = ... | taint.cpp:753:5:753:5 | x [post update] | |
6618+
| taint.cpp:753:9:753:14 | call to source | taint.cpp:753:2:753:16 | ... = ... | |
6619+
| taint.cpp:754:14:754:20 | call to realloc | taint.cpp:755:7:755:8 | a2 | |
6620+
| taint.cpp:754:22:754:22 | a | taint.cpp:754:14:754:20 | call to realloc | TAINT |
6621+
| taint.cpp:760:31:760:34 | path | taint.cpp:760:31:760:34 | path | |
6622+
| taint.cpp:760:31:760:34 | path | taint.cpp:761:10:761:13 | path | |
6623+
| taint.cpp:760:31:760:34 | path | taint.cpp:762:10:762:13 | path | |
6624+
| taint.cpp:760:43:760:46 | data | taint.cpp:760:43:760:46 | data | |
6625+
| taint.cpp:760:43:760:46 | data | taint.cpp:762:22:762:25 | data | |
6626+
| taint.cpp:761:10:761:13 | ref arg path | taint.cpp:760:31:760:34 | path | |
6627+
| taint.cpp:761:10:761:13 | ref arg path | taint.cpp:762:10:762:13 | path | |
6628+
| taint.cpp:761:16:761:19 | %s | taint.cpp:761:10:761:13 | ref arg path | TAINT |
6629+
| taint.cpp:761:22:761:26 | abc | taint.cpp:761:10:761:13 | ref arg path | TAINT |
6630+
| taint.cpp:762:10:762:13 | ref arg path | taint.cpp:760:31:760:34 | path | |
6631+
| taint.cpp:762:16:762:19 | %s | taint.cpp:762:10:762:13 | ref arg path | TAINT |
6632+
| taint.cpp:762:22:762:25 | data | taint.cpp:762:10:762:13 | ref arg path | TAINT |
6633+
| taint.cpp:762:22:762:25 | ref arg data | taint.cpp:760:43:760:46 | data | |
6634+
| taint.cpp:766:7:766:10 | path | taint.cpp:767:21:767:24 | path | |
6635+
| taint.cpp:766:7:766:10 | path | taint.cpp:768:8:768:11 | path | |
6636+
| taint.cpp:767:21:767:24 | ref arg path | taint.cpp:768:8:768:11 | path | |
6637+
| taint.cpp:768:8:768:11 | path | taint.cpp:768:7:768:11 | * ... | |
6638+
| taint.cpp:778:37:778:42 | call to source | taint.cpp:779:7:779:9 | obj | |
66326639
| vector.cpp:16:43:16:49 | source1 | vector.cpp:17:26:17:32 | source1 | |
66336640
| vector.cpp:16:43:16:49 | source1 | vector.cpp:31:38:31:44 | source1 | |
66346641
| vector.cpp:17:21:17:33 | call to vector | vector.cpp:19:14:19:14 | v | |

0 commit comments

Comments
 (0)