Skip to content

Commit fe68255

Browse files
MathiasVPMathiasVP
authored
Merge pull request github#3475 from jbj/field-conflation-memcpy
C++: Test field conflation with array in struct
2 parents efa133f + a380dc1 commit fe68255

File tree

5 files changed

+78
-1
lines changed

5 files changed

+78
-1
lines changed

‎cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/defaulttainttracking.cpp

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,4 +96,24 @@ void test_outparams() {
9696
char *p2 = nullptr;
9797
flow_to_outparam(&p2, getenv("VAR"));
9898
sink(p2); // tainted
99-
}
99+
}
100+
101+
102+
void *memcpy(void *dst, void *src, int size);
103+
104+
struct ContainsArray {
105+
int arr[16];
106+
int x;
107+
};
108+
109+
void taint_array(ContainsArray *ca, int offset) {
110+
int tainted = getenv("VAR")[0];
111+
memcpy(ca->arr + offset, &tainted, sizeof(int));
112+
}
113+
114+
void test_conflated_fields3(int arbitrary) {
115+
ContainsArray ca;
116+
ca.x = 0;
117+
taint_array(&ca, arbitrary);
118+
sink(ca.x); // not tainted [FALSE POSITIVE]
119+
}

‎cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/tainted.expected

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,17 @@
109109
| defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:98:10:98:11 | (const char *)... |
110110
| defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:98:10:98:11 | p2 |
111111
| defaulttainttracking.cpp:97:27:97:32 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 |
112+
| defaulttainttracking.cpp:110:17:110:22 | call to getenv | defaulttainttracking.cpp:10:11:10:13 | p#0 |
113+
| defaulttainttracking.cpp:110:17:110:22 | call to getenv | defaulttainttracking.cpp:102:31:102:33 | src |
114+
| defaulttainttracking.cpp:110:17:110:22 | call to getenv | defaulttainttracking.cpp:110:7:110:13 | tainted |
115+
| defaulttainttracking.cpp:110:17:110:22 | call to getenv | defaulttainttracking.cpp:110:17:110:22 | call to getenv |
116+
| defaulttainttracking.cpp:110:17:110:22 | call to getenv | defaulttainttracking.cpp:110:17:110:32 | (int)... |
117+
| defaulttainttracking.cpp:110:17:110:22 | call to getenv | defaulttainttracking.cpp:110:17:110:32 | access to array |
118+
| defaulttainttracking.cpp:110:17:110:22 | call to getenv | defaulttainttracking.cpp:111:3:111:8 | call to memcpy |
119+
| defaulttainttracking.cpp:110:17:110:22 | call to getenv | defaulttainttracking.cpp:111:28:111:35 | & ... |
120+
| defaulttainttracking.cpp:110:17:110:22 | call to getenv | defaulttainttracking.cpp:111:28:111:35 | (void *)... |
121+
| defaulttainttracking.cpp:110:17:110:22 | call to getenv | defaulttainttracking.cpp:118:11:118:11 | x |
122+
| defaulttainttracking.cpp:110:17:110:22 | call to getenv | test_diff.cpp:2:11:2:13 | p#0 |
112123
| globals.cpp:5:20:5:25 | call to getenv | globals.cpp:2:17:2:25 | sinkParam |
113124
| globals.cpp:5:20:5:25 | call to getenv | globals.cpp:5:12:5:16 | local |
114125
| globals.cpp:5:20:5:25 | call to getenv | globals.cpp:5:20:5:25 | call to getenv |

‎cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/test_diff.expected

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@
2323
| defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:98:10:98:11 | (const char *)... | IR only |
2424
| defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:98:10:98:11 | p2 | IR only |
2525
| defaulttainttracking.cpp:97:27:97:32 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 | IR only |
26+
| defaulttainttracking.cpp:110:17:110:22 | call to getenv | defaulttainttracking.cpp:10:11:10:13 | p#0 | IR only |
27+
| defaulttainttracking.cpp:110:17:110:22 | call to getenv | defaulttainttracking.cpp:102:20:102:22 | dst | AST only |
28+
| defaulttainttracking.cpp:110:17:110:22 | call to getenv | defaulttainttracking.cpp:111:10:111:25 | ... + ... | AST only |
29+
| defaulttainttracking.cpp:110:17:110:22 | call to getenv | defaulttainttracking.cpp:111:29:111:35 | tainted | AST only |
30+
| defaulttainttracking.cpp:110:17:110:22 | call to getenv | defaulttainttracking.cpp:118:11:118:11 | x | IR only |
31+
| defaulttainttracking.cpp:110:17:110:22 | call to getenv | test_diff.cpp:2:11:2:13 | p#0 | IR only |
2632
| globals.cpp:13:15:13:20 | call to getenv | globals.cpp:13:5:13:11 | global1 | AST only |
2733
| globals.cpp:23:15:23:20 | call to getenv | globals.cpp:23:5:23:11 | global2 | AST only |
2834
| test_diff.cpp:104:12:104:15 | argv | test_diff.cpp:104:11:104:20 | (...) | IR only |

‎cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/TaintedAllocationSize/TaintedAllocationSize.expected

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,13 @@
11
edges
2+
| field_conflation.c:12:22:12:27 | call to getenv | field_conflation.c:13:10:13:25 | Chi |
3+
| field_conflation.c:12:22:12:34 | (const char *)... | field_conflation.c:13:10:13:25 | Chi |
4+
| field_conflation.c:13:10:13:25 | Chi | field_conflation.c:19:15:19:17 | taint_array output argument |
5+
| field_conflation.c:19:15:19:17 | taint_array output argument | field_conflation.c:20:10:20:13 | (unsigned long)... |
6+
| field_conflation.c:19:15:19:17 | taint_array output argument | field_conflation.c:20:13:20:13 | x |
7+
| field_conflation.c:19:15:19:17 | taint_array output argument | field_conflation.c:20:13:20:13 | x |
8+
| field_conflation.c:19:15:19:17 | taint_array output argument | field_conflation.c:20:13:20:13 | x |
9+
| field_conflation.c:20:13:20:13 | x | field_conflation.c:20:10:20:13 | (unsigned long)... |
10+
| field_conflation.c:20:13:20:13 | x | field_conflation.c:20:13:20:13 | x |
211
| test.cpp:39:21:39:24 | argv | test.cpp:42:38:42:44 | (size_t)... |
312
| test.cpp:39:21:39:24 | argv | test.cpp:42:38:42:44 | (size_t)... |
413
| test.cpp:39:21:39:24 | argv | test.cpp:42:38:42:44 | tainted |
@@ -60,6 +69,15 @@ edges
6069
| test.cpp:235:11:235:20 | (size_t)... | test.cpp:214:23:214:23 | s |
6170
| test.cpp:237:10:237:19 | (size_t)... | test.cpp:220:21:220:21 | s |
6271
nodes
72+
| field_conflation.c:12:22:12:27 | call to getenv | semmle.label | call to getenv |
73+
| field_conflation.c:12:22:12:34 | (const char *)... | semmle.label | (const char *)... |
74+
| field_conflation.c:13:10:13:25 | Chi | semmle.label | Chi |
75+
| field_conflation.c:19:15:19:17 | taint_array output argument | semmle.label | taint_array output argument |
76+
| field_conflation.c:20:10:20:13 | (unsigned long)... | semmle.label | (unsigned long)... |
77+
| field_conflation.c:20:10:20:13 | (unsigned long)... | semmle.label | (unsigned long)... |
78+
| field_conflation.c:20:13:20:13 | x | semmle.label | x |
79+
| field_conflation.c:20:13:20:13 | x | semmle.label | x |
80+
| field_conflation.c:20:13:20:13 | x | semmle.label | x |
6381
| test.cpp:39:21:39:24 | argv | semmle.label | argv |
6482
| test.cpp:39:21:39:24 | argv | semmle.label | argv |
6583
| test.cpp:42:38:42:44 | (size_t)... | semmle.label | (size_t)... |
@@ -123,6 +141,7 @@ nodes
123141
| test.cpp:235:11:235:20 | (size_t)... | semmle.label | (size_t)... |
124142
| test.cpp:237:10:237:19 | (size_t)... | semmle.label | (size_t)... |
125143
#select
144+
| field_conflation.c:20:3:20:8 | call to malloc | field_conflation.c:12:22:12:27 | call to getenv | field_conflation.c:20:13:20:13 | x | This allocation size is derived from $@ and might overflow | field_conflation.c:12:22:12:27 | call to getenv | user input (getenv) |
126145
| test.cpp:42:31:42:36 | call to malloc | test.cpp:39:21:39:24 | argv | test.cpp:42:38:42:44 | tainted | This allocation size is derived from $@ and might overflow | test.cpp:39:21:39:24 | argv | user input (argv) |
127146
| test.cpp:43:31:43:36 | call to malloc | test.cpp:39:21:39:24 | argv | test.cpp:43:38:43:63 | ... * ... | This allocation size is derived from $@ and might overflow | test.cpp:39:21:39:24 | argv | user input (argv) |
128147
| test.cpp:45:31:45:36 | call to malloc | test.cpp:39:21:39:24 | argv | test.cpp:45:38:45:63 | ... + ... | This allocation size is derived from $@ and might overflow | test.cpp:39:21:39:24 | argv | user input (argv) |

‎cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/TaintedAllocationSize/field_conflation.c

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
int atoi(const char *nptr);
2+
void *malloc(unsigned long size);
3+
char *getenv(const char *name);
4+
void *memcpy(void *dst, void *src, unsigned long size);
5+
6+
struct ContainsArray {
7+
int arr[16];
8+
int x;
9+
};
10+
11+
void taint_array(struct ContainsArray *ca, int offset) {
12+
int tainted = atoi(getenv("VAR"));
13+
memcpy(ca->arr + offset, &tainted, sizeof(int));
14+
}
15+
16+
void test_conflated_fields3(int arbitrary) {
17+
struct ContainsArray ca;
18+
ca.x = 4;
19+
taint_array(&ca, arbitrary);
20+
malloc(ca.x); // not tainted [FALSE POSITIVE]
21+
}

0 commit comments

Comments
 (0)