Skip to content

Commit 3632d51

Browse files
jbjjbj
authored
Merge pull request #2635 from geoffw0/modelstrdup
CPP: Model strdup
2 parents c7e62b4 + 7dbda22 commit 3632d51

9 files changed

Lines changed: 149 additions & 21 deletions

File tree

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,7 @@ The following changes in version 1.24 affect C/C++ analysis in all applications.
3535
about the _name or scope_ of variables should remain unchanged.
3636
* The `LocalScopeVariableReachability` library is deprecated in favor of
3737
`StackVariableReachability`. The functionality is the same.
38+
* The taint tracking library (`semmle.code.cpp.dataflow.TaintTracking`) has had
39+
the following improvements:
40+
* The library now models data flow through `strdup` and similar functions.
41+

‎cpp/ql/src/semmle/code/cpp/dataflow/internal/TaintTrackingUtil.qll‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ predicate localAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeT
6969
or
7070
// Taint can flow through modeled functions
7171
exprToDefinitionByReferenceStep(nodeFrom.asExpr(), nodeTo.asDefiningArgument())
72+
or
73+
exprToExprStep(nodeFrom.asExpr(), nodeTo.asExpr())
7274
}
7375

7476
/**
@@ -118,6 +120,36 @@ private predicate noFlowFromChildExpr(Expr e) {
118120
e instanceof FieldAccess
119121
}
120122

123+
private predicate exprToExprStep(Expr exprIn, Expr exprOut) {
124+
exists(DataFlowFunction f, Call call, FunctionOutput outModel |
125+
call.getTarget() = f and
126+
exprOut = call and
127+
outModel.isReturnValueDeref() and
128+
exists(int argInIndex, FunctionInput inModel | f.hasDataFlow(inModel, outModel) |
129+
// Taint flows from a pointer to a dereference, which DataFlow does not handle
130+
// dest_ptr = strdup(tainted_ptr)
131+
inModel.isParameterDeref(argInIndex) and
132+
exprIn = call.getArgument(argInIndex)
133+
)
134+
)
135+
or
136+
exists(TaintFunction f, Call call, FunctionOutput outModel |
137+
call.getTarget() = f and
138+
exprOut = call and
139+
outModel.isReturnValueDeref() and
140+
exists(int argInIndex, FunctionInput inModel | f.hasTaintFlow(inModel, outModel) |
141+
inModel.isParameterDeref(argInIndex) and
142+
exprIn = call.getArgument(argInIndex)
143+
or
144+
inModel.isParameterDeref(argInIndex) and
145+
call.passesByReference(argInIndex, exprIn)
146+
or
147+
inModel.isParameter(argInIndex) and
148+
exprIn = call.getArgument(argInIndex)
149+
)
150+
)
151+
}
152+
121153
private predicate exprToDefinitionByReferenceStep(Expr exprIn, Expr argOut) {
122154
exists(DataFlowFunction f, Call call, FunctionOutput outModel, int argOutIndex |
123155
call.getTarget() = f and

‎cpp/ql/src/semmle/code/cpp/models/Models.qll‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,6 @@ private import implementations.Printf
99
private import implementations.Pure
1010
private import implementations.Strcat
1111
private import implementations.Strcpy
12+
private import implementations.Strdup
1213
private import implementations.Strftime
1314
private import implementations.Swap

‎cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll‎

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -173,32 +173,14 @@ class ReallocAllocationFunction extends AllocationFunction {
173173
}
174174

175175
/**
176-
* An allocation function (such as `strdup`) that has no explicit argument for
176+
* A miscellaneous allocation function that has no explicit argument for
177177
* the size of the allocation.
178178
*/
179-
class StrdupAllocationFunction extends AllocationFunction {
180-
StrdupAllocationFunction() {
179+
class SizelessAllocationFunction extends AllocationFunction {
180+
SizelessAllocationFunction() {
181181
exists(string name |
182-
hasGlobalOrStdName(name) and
183-
(
184-
// strdup(str)
185-
name = "strdup"
186-
or
187-
// wcsdup(str)
188-
name = "wcsdup"
189-
)
190-
or
191182
hasGlobalName(name) and
192183
(
193-
// _strdup(str)
194-
name = "_strdup"
195-
or
196-
// _wcsdup(str)
197-
name = "_wcsdup"
198-
or
199-
// _mbsdup(str)
200-
name = "_mbsdup"
201-
or
202184
// ExAllocateFromLookasideListEx(list)
203185
name = "ExAllocateFromLookasideListEx"
204186
or

‎cpp/ql/src/semmle/code/cpp/models/implementations/Strdup.qll‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import semmle.code.cpp.models.interfaces.Allocation
2+
import semmle.code.cpp.models.interfaces.ArrayFunction
3+
import semmle.code.cpp.models.interfaces.DataFlow
4+
import semmle.code.cpp.models.interfaces.Taint
5+
6+
/**
7+
* A `strdup` style allocation function.
8+
*/
9+
class StrdupFunction extends AllocationFunction, ArrayFunction, DataFlowFunction {
10+
StrdupFunction() {
11+
exists(string name |
12+
hasGlobalOrStdName(name) and
13+
(
14+
// strdup(str)
15+
name = "strdup"
16+
or
17+
// wcsdup(str)
18+
name = "wcsdup"
19+
)
20+
or
21+
hasGlobalName(name) and
22+
(
23+
// _strdup(str)
24+
name = "_strdup"
25+
or
26+
// _wcsdup(str)
27+
name = "_wcsdup"
28+
or
29+
// _mbsdup(str)
30+
name = "_mbsdup"
31+
)
32+
)
33+
}
34+
35+
override predicate hasArrayInput(int bufParam) { bufParam = 0 }
36+
37+
override predicate hasArrayWithNullTerminator(int bufParam) { bufParam = 0 }
38+
39+
override predicate hasDataFlow(FunctionInput input, FunctionOutput output) {
40+
// These always copy the full value of the input buffer to the result
41+
// buffer
42+
input.isParameterDeref(0) and
43+
output.isReturnValueDeref()
44+
}
45+
}

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@
144144
| taint.cpp:170:10:170:15 | ref arg buffer | taint.cpp:171:8:171:13 | buffer | |
145145
| taint.cpp:170:10:170:15 | ref arg buffer | taint.cpp:172:10:172:15 | buffer | |
146146
| taint.cpp:170:10:170:15 | ref arg buffer | taint.cpp:173:8:173:13 | buffer | |
147+
| taint.cpp:170:18:170:26 | Hello, | taint.cpp:170:3:170:8 | call to strcpy | TAINT |
147148
| taint.cpp:170:18:170:26 | Hello, | taint.cpp:170:10:170:15 | ref arg buffer | TAINT |
148149
| taint.cpp:171:8:171:13 | ref arg buffer | taint.cpp:172:10:172:15 | buffer | |
149150
| taint.cpp:171:8:171:13 | ref arg buffer | taint.cpp:173:8:173:13 | buffer | |
@@ -163,7 +164,9 @@
163164
| taint.cpp:194:9:194:10 | ref arg & ... | taint.cpp:194:2:194:7 | call to memcpy | |
164165
| taint.cpp:194:9:194:10 | ref arg & ... | taint.cpp:195:7:195:7 | x | |
165166
| taint.cpp:194:10:194:10 | x | taint.cpp:194:9:194:10 | & ... | |
167+
| taint.cpp:194:13:194:18 | source | taint.cpp:194:2:194:7 | call to memcpy | TAINT |
166168
| taint.cpp:194:13:194:18 | source | taint.cpp:194:9:194:10 | ref arg & ... | TAINT |
169+
| taint.cpp:194:21:194:31 | sizeof(int) | taint.cpp:194:2:194:7 | call to memcpy | TAINT |
167170
| taint.cpp:194:21:194:31 | sizeof(int) | taint.cpp:194:9:194:10 | ref arg & ... | TAINT |
168171
| taint.cpp:207:6:207:11 | call to source | taint.cpp:207:2:207:13 | ... = ... | |
169172
| taint.cpp:207:6:207:11 | call to source | taint.cpp:210:7:210:7 | x | |
@@ -324,3 +327,23 @@
324327
| taint.cpp:347:13:347:13 | d | taint.cpp:347:12:347:13 | & ... | |
325328
| taint.cpp:348:14:348:14 | ref arg e | taint.cpp:355:7:355:7 | e | |
326329
| taint.cpp:348:17:348:17 | ref arg t | taint.cpp:350:7:350:7 | t | |
330+
| taint.cpp:365:24:365:29 | source | taint.cpp:369:13:369:18 | source | |
331+
| taint.cpp:365:24:365:29 | source | taint.cpp:371:14:371:19 | source | |
332+
| taint.cpp:369:6:369:11 | call to strdup | taint.cpp:369:2:369:19 | ... = ... | |
333+
| taint.cpp:369:6:369:11 | call to strdup | taint.cpp:372:7:372:7 | a | |
334+
| taint.cpp:369:13:369:18 | source | taint.cpp:369:6:369:11 | call to strdup | TAINT |
335+
| taint.cpp:370:6:370:11 | call to strdup | taint.cpp:370:2:370:27 | ... = ... | |
336+
| taint.cpp:370:6:370:11 | call to strdup | taint.cpp:373:7:373:7 | b | |
337+
| taint.cpp:370:13:370:26 | hello, world | taint.cpp:370:6:370:11 | call to strdup | TAINT |
338+
| taint.cpp:371:6:371:12 | call to strndup | taint.cpp:371:2:371:25 | ... = ... | |
339+
| taint.cpp:371:6:371:12 | call to strndup | taint.cpp:374:7:374:7 | c | |
340+
| taint.cpp:377:23:377:28 | source | taint.cpp:381:30:381:35 | source | |
341+
| taint.cpp:381:6:381:12 | call to strndup | taint.cpp:381:2:381:36 | ... = ... | |
342+
| taint.cpp:381:6:381:12 | call to strndup | taint.cpp:382:7:382:7 | a | |
343+
| taint.cpp:385:27:385:32 | source | taint.cpp:389:13:389:18 | source | |
344+
| taint.cpp:389:6:389:11 | call to wcsdup | taint.cpp:389:2:389:19 | ... = ... | |
345+
| taint.cpp:389:6:389:11 | call to wcsdup | taint.cpp:391:7:391:7 | a | |
346+
| taint.cpp:389:13:389:18 | source | taint.cpp:389:6:389:11 | call to wcsdup | TAINT |
347+
| taint.cpp:390:6:390:11 | call to wcsdup | taint.cpp:390:2:390:28 | ... = ... | |
348+
| taint.cpp:390:6:390:11 | call to wcsdup | taint.cpp:392:7:392:7 | b | |
349+
| taint.cpp:390:13:390:27 | hello, world | taint.cpp:390:6:390:11 | call to wcsdup | TAINT |

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,3 +354,40 @@ void test_outparams()
354354
sink(d); // tainted [NOT DETECTED]
355355
sink(e);
356356
}
357+
358+
// --- strdup ---
359+
360+
typedef unsigned long size_t;
361+
char *strdup(const char *s1);
362+
char *strndup(const char *s1, size_t n);
363+
wchar_t* wcsdup(const wchar_t* s1);
364+
365+
void test_strdup(char *source)
366+
{
367+
char *a, *b, *c;
368+
369+
a = strdup(source);
370+
b = strdup("hello, world");
371+
c = strndup(source, 100);
372+
sink(a); // tainted
373+
sink(b);
374+
sink(c); // tainted [NOT DETECTED]
375+
}
376+
377+
void test_strndup(int source)
378+
{
379+
char *a;
380+
381+
a = strndup("hello, world", source);
382+
sink(a);
383+
}
384+
385+
void test_wcsdup(wchar_t *source)
386+
{
387+
wchar_t *a, *b;
388+
389+
a = wcsdup(source);
390+
b = wcsdup(L"hello, world");
391+
sink(a); // tainted
392+
sink(b);
393+
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,3 +37,5 @@
3737
| taint.cpp:350:7:350:7 | t | taint.cpp:330:6:330:11 | call to source |
3838
| taint.cpp:351:7:351:7 | a | taint.cpp:330:6:330:11 | call to source |
3939
| taint.cpp:352:7:352:7 | b | taint.cpp:330:6:330:11 | call to source |
40+
| taint.cpp:372:7:372:7 | a | taint.cpp:365:24:365:29 | source |
41+
| taint.cpp:391:7:391:7 | a | taint.cpp:385:27:385:32 | source |

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,5 @@
2424
| taint.cpp:261:7:261:7 | taint.cpp:258:7:258:12 | AST only |
2525
| taint.cpp:351:7:351:7 | taint.cpp:330:6:330:11 | AST only |
2626
| taint.cpp:352:7:352:7 | taint.cpp:330:6:330:11 | AST only |
27+
| taint.cpp:372:7:372:7 | taint.cpp:365:24:365:29 | AST only |
28+
| taint.cpp:391:7:391:7 | taint.cpp:385:27:385:32 | AST only |

0 commit comments

Comments
 (0)