Skip to content

Commit bdb678a

Browse files
jbjjbj
authored
Merge pull request #1267 from rdmarsh2/rdmarsh/cpp/def-by-ref-taint
C++: add taint edges to DefinitionByReferenceNode
2 parents 2db06f9 + f5c57b7 commit bdb678a

File tree

6 files changed

+69
-2
lines changed

6 files changed

+69
-2
lines changed

‎change-notes/1.21/analysis-cpp.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,6 @@
2626
| Wrong type of arguments to formatting function (`cpp/wrong-type-format-argument`) | Fewer false positive results | Non-standard uses of %L are now understood. |
2727

2828
## Changes to QL libraries
29+
- Additional support for definition by reference has been added to the `semmle.code.cpp.dataflow.TaintTracking` library.
30+
- The taint tracking library now includes taint-specific edges for functions modeled in `semmle.code.cpp.models.interfaces.DataFlow`.
31+
- The taint tracking library adds flow through library functions that are modeled in `semmle.code.cpp.models.interfaces.Taint`. Queries can add subclasses of `TaintFunction` to specify additional flow.

‎cpp/ql/src/semmle/code/cpp/dataflow/TaintTracking.qll‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
*/
1010
import semmle.code.cpp.dataflow.DataFlow
1111
import semmle.code.cpp.dataflow.DataFlow2
12+
import semmle.code.cpp.models.interfaces.DataFlow
13+
import semmle.code.cpp.models.interfaces.Taint
1214

1315
module TaintTracking {
1416

@@ -187,6 +189,9 @@ module TaintTracking {
187189
exprFrom.(PostfixCrementOperation)
188190
)
189191
)
192+
or
193+
// Taint can flow through modeled functions
194+
exprToDefinitionByReferenceStep(nodeFrom.asExpr(), nodeTo.asDefiningArgument())
190195
}
191196

192197
/**
@@ -226,4 +231,37 @@ module TaintTracking {
226231
e instanceof AlignofOperator
227232
}
228233

229-
}
234+
private predicate exprToDefinitionByReferenceStep(Expr exprIn, Expr argOut) {
235+
exists(DataFlowFunction f, Call call, FunctionOutput outModel, int argOutIndex |
236+
call.getTarget() = f and
237+
argOut = call.getArgument(argOutIndex) and
238+
outModel.isOutParameterPointer(argOutIndex) and
239+
exists(int argInIndex, FunctionInput inModel |
240+
f.hasDataFlow(inModel, outModel)
241+
|
242+
// Taint flows from a pointer to a dereference, which DataFlow does not handle
243+
// memcpy(&dest_var, tainted_ptr, len)
244+
inModel.isInParameterPointer(argInIndex) and
245+
exprIn = call.getArgument(argInIndex)
246+
)
247+
)
248+
or
249+
exists(TaintFunction f, Call call, FunctionOutput outModel, int argOutIndex |
250+
call.getTarget() = f and
251+
argOut = call.getArgument(argOutIndex) and
252+
outModel.isOutParameterPointer(argOutIndex) and
253+
exists(int argInIndex, FunctionInput inModel |
254+
f.hasTaintFlow(inModel, outModel)
255+
|
256+
inModel.isInParameterPointer(argInIndex) and
257+
exprIn = call.getArgument(argInIndex)
258+
or
259+
inModel.isInParameterPointer(argInIndex) and
260+
call.passesByReference(argInIndex, exprIn)
261+
or
262+
inModel.isInParameter(argInIndex) and
263+
exprIn = call.getArgument(argInIndex)
264+
)
265+
)
266+
}
267+
}

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,15 +137,27 @@
137137
| taint.cpp:170:10:170:15 | ref arg buffer | taint.cpp:171:8:171:13 | buffer | |
138138
| taint.cpp:170:10:170:15 | ref arg buffer | taint.cpp:172:10:172:15 | buffer | |
139139
| taint.cpp:170:10:170:15 | ref arg buffer | taint.cpp:173:8:173:13 | buffer | |
140+
| taint.cpp:170:18:170:26 | Hello, | taint.cpp:170:10:170:15 | ref arg buffer | TAINT |
140141
| taint.cpp:171:8:171:13 | ref arg buffer | taint.cpp:171:8:171:13 | buffer | |
141142
| taint.cpp:171:8:171:13 | ref arg buffer | taint.cpp:172:10:172:15 | buffer | |
142143
| taint.cpp:171:8:171:13 | ref arg buffer | taint.cpp:173:8:173:13 | buffer | |
143144
| taint.cpp:172:10:172:15 | buffer | taint.cpp:172:3:172:8 | call to strcat | |
145+
| taint.cpp:172:10:172:15 | buffer | taint.cpp:172:10:172:15 | ref arg buffer | TAINT |
144146
| taint.cpp:172:10:172:15 | ref arg buffer | taint.cpp:172:10:172:15 | buffer | |
145147
| taint.cpp:172:10:172:15 | ref arg buffer | taint.cpp:173:8:173:13 | buffer | |
148+
| taint.cpp:172:18:172:24 | tainted | taint.cpp:172:10:172:15 | ref arg buffer | TAINT |
146149
| taint.cpp:173:8:173:13 | ref arg buffer | taint.cpp:173:8:173:13 | buffer | |
147150
| taint.cpp:180:19:180:19 | p | taint.cpp:181:9:181:9 | p | |
148151
| taint.cpp:181:9:181:9 | p | taint.cpp:181:8:181:9 | * ... | TAINT |
149152
| taint.cpp:185:11:185:16 | call to source | taint.cpp:186:11:186:11 | x | |
150153
| taint.cpp:186:10:186:11 | ref arg & ... | taint.cpp:186:11:186:11 | x | |
151154
| taint.cpp:186:11:186:11 | x | taint.cpp:186:10:186:11 | & ... | TAINT |
155+
| taint.cpp:192:23:192:28 | source | taint.cpp:194:13:194:18 | source | |
156+
| taint.cpp:193:6:193:6 | x | taint.cpp:194:10:194:10 | x | |
157+
| taint.cpp:193:6:193:6 | x | taint.cpp:195:7:195:7 | x | |
158+
| taint.cpp:194:9:194:10 | & ... | taint.cpp:194:2:194:7 | call to memcpy | |
159+
| taint.cpp:194:9:194:10 | ref arg & ... | taint.cpp:194:10:194:10 | x | |
160+
| taint.cpp:194:9:194:10 | ref arg & ... | taint.cpp:195:7:195:7 | x | |
161+
| taint.cpp:194:10:194:10 | x | taint.cpp:194:9:194:10 | & ... | TAINT |
162+
| taint.cpp:194:13:194:18 | source | taint.cpp:194:9:194:10 | ref arg & ... | TAINT |
163+
| taint.cpp:194:21:194:31 | sizeof(int) | taint.cpp:194:9:194:10 | ref arg & ... | TAINT |

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ namespace strings
170170
strcpy(buffer, "Hello, ");
171171
sink(buffer);
172172
strcat(buffer, tainted);
173-
sink(buffer); // tainted [NOT DETECTED]
173+
sink(buffer); // tainted
174174
}
175175
}
176176

@@ -186,3 +186,11 @@ namespace refs {
186186
callee(&x);
187187
}
188188
}
189+
190+
void *memcpy(void *dest, void *src, int len);
191+
192+
void test_memcpy(int *source) {
193+
int x;
194+
memcpy(&x, source, sizeof(int));
195+
sink(x);
196+
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,7 @@
1010
| taint.cpp:151:7:151:12 | call to select | taint.cpp:151:20:151:25 | call to source |
1111
| taint.cpp:167:8:167:13 | call to source | taint.cpp:167:8:167:13 | call to source |
1212
| taint.cpp:168:8:168:14 | tainted | taint.cpp:164:19:164:24 | call to source |
13+
| taint.cpp:173:8:173:13 | buffer | taint.cpp:164:19:164:24 | call to source |
1314
| taint.cpp:181:8:181:9 | * ... | taint.cpp:185:11:185:16 | call to source |
15+
| taint.cpp:195:7:195:7 | x | taint.cpp:192:23:192:28 | source |
16+
| taint.cpp:195:7:195:7 | x | taint.cpp:193:6:193:6 | x |

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,7 @@
33
| taint.cpp:37:22:37:27 | taint.cpp:43:7:43:13 | AST only |
44
| taint.cpp:120:11:120:16 | taint.cpp:137:7:137:9 | AST only |
55
| taint.cpp:127:8:127:13 | taint.cpp:130:7:130:9 | IR only |
6+
| taint.cpp:164:19:164:24 | taint.cpp:173:8:173:13 | AST only |
67
| taint.cpp:185:11:185:16 | taint.cpp:181:8:181:9 | AST only |
8+
| taint.cpp:192:23:192:28 | taint.cpp:195:7:195:7 | AST only |
9+
| taint.cpp:193:6:193:6 | taint.cpp:195:7:195:7 | AST only |

0 commit comments

Comments
 (0)