Skip to content

Commit 342b3d7

Browse files
MathiasVPMathiasVP
authored
Merge pull request github#14619 from MathiasVP/fix-strtol-model
C++: Fix `strtol` model
2 parents c1ecd5a + 33494fe commit 342b3d7

File tree

10 files changed

+222
-3
lines changed

10 files changed

+222
-3
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ private import implementations.Strtok
1919
private import implementations.Strset
2020
private import implementations.Strcrement
2121
private import implementations.Strnextc
22+
private import implementations.Strtol
2223
private import implementations.StdContainer
2324
private import implementations.StdPair
2425
private import implementations.StdMap

‎cpp/ql/lib/semmle/code/cpp/models/implementations/Pure.qll

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ private class PureStrFunction extends AliasFunction, ArrayFunction, TaintFunctio
1313
PureStrFunction() {
1414
this.hasGlobalOrStdOrBslName([
1515
atoi(), "strcasestr", "strchnul", "strchr", "strchrnul", "strstr", "strpbrk", "strrchr",
16-
"strspn", strtol(), strrev(), strcmp(), strlwr(), strupr()
16+
"strspn", strrev(), strcmp(), strlwr(), strupr()
1717
])
1818
}
1919

@@ -70,8 +70,6 @@ private class PureStrFunction extends AliasFunction, ArrayFunction, TaintFunctio
7070

7171
private string atoi() { result = ["atof", "atoi", "atol", "atoll"] }
7272

73-
private string strtol() { result = ["strtod", "strtof", "strtol", "strtoll", "strtoq", "strtoul"] }
74-
7573
private string strlwr() {
7674
result = ["_strlwr", "_wcslwr", "_mbslwr", "_strlwr_l", "_wcslwr_l", "_mbslwr_l"]
7775
}

‎cpp/ql/lib/semmle/code/cpp/models/implementations/Strtol.qll

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import semmle.code.cpp.models.interfaces.ArrayFunction
2+
import semmle.code.cpp.models.interfaces.Taint
3+
import semmle.code.cpp.models.interfaces.Alias
4+
import semmle.code.cpp.models.interfaces.SideEffect
5+
6+
private string strtol() { result = ["strtod", "strtof", "strtol", "strtoll", "strtoq", "strtoul"] }
7+
8+
/**
9+
* The standard function `strtol` and its assorted variants
10+
*/
11+
private class Strtol extends AliasFunction, ArrayFunction, TaintFunction, SideEffectFunction {
12+
Strtol() { this.hasGlobalOrStdOrBslName(strtol()) }
13+
14+
override predicate hasArrayInput(int bufParam) {
15+
// All the functions given by `strtol()` takes a `const char*` input as the first parameter
16+
bufParam = 0
17+
}
18+
19+
override predicate hasArrayWithNullTerminator(int bufParam) { bufParam = 0 }
20+
21+
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
22+
(
23+
input.isParameter(0)
24+
or
25+
input.isParameterDeref(0)
26+
) and
27+
output.isReturnValue()
28+
or
29+
input.isParameter(0) and
30+
output.isParameterDeref(1)
31+
}
32+
33+
override predicate parameterNeverEscapes(int i) {
34+
// Parameter 0 does escape into parameter 1.
35+
i = 1
36+
}
37+
38+
override predicate parameterEscapesOnlyViaReturn(int i) { none() }
39+
40+
override predicate parameterIsAlwaysReturned(int i) { none() }
41+
42+
override predicate hasOnlySpecificReadSideEffects() { any() }
43+
44+
override predicate hasOnlySpecificWriteSideEffects() { any() }
45+
46+
override predicate hasSpecificReadSideEffect(ParameterIndex i, boolean buffer) {
47+
i = 0 and
48+
buffer = true
49+
}
50+
51+
override predicate hasSpecificWriteSideEffect(ParameterIndex i, boolean buffer, boolean mustWrite) {
52+
i = 1 and buffer = false and mustWrite = false
53+
}
54+
}

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6629,6 +6629,20 @@ WARNING: Module TaintTracking has been deprecated and may be removed in future (
66296629
| taint.cpp:720:27:720:32 | source | taint.cpp:720:20:720:25 | call to strtok | TAINT |
66306630
| taint.cpp:721:8:721:16 | tokenized | taint.cpp:721:7:721:16 | * ... | TAINT |
66316631
| taint.cpp:722:8:722:12 | delim | taint.cpp:722:7:722:12 | * ... | TAINT |
6632+
| taint.cpp:727:24:727:29 | source | taint.cpp:727:24:727:29 | source | |
6633+
| taint.cpp:727:24:727:29 | source | taint.cpp:729:18:729:23 | source | |
6634+
| taint.cpp:728:17:728:23 | 0 | taint.cpp:729:27:729:32 | endptr | |
6635+
| taint.cpp:728:17:728:23 | 0 | taint.cpp:731:7:731:12 | endptr | |
6636+
| taint.cpp:728:17:728:23 | 0 | taint.cpp:732:8:732:13 | endptr | |
6637+
| taint.cpp:729:11:729:16 | call to strtol | taint.cpp:730:7:730:7 | l | |
6638+
| taint.cpp:729:18:729:23 | source | taint.cpp:729:11:729:16 | call to strtol | TAINT |
6639+
| taint.cpp:729:18:729:23 | source | taint.cpp:729:26:729:32 | ref arg & ... | TAINT |
6640+
| taint.cpp:729:26:729:32 | ref arg & ... | taint.cpp:729:27:729:32 | endptr [inner post update] | |
6641+
| taint.cpp:729:26:729:32 | ref arg & ... | taint.cpp:731:7:731:12 | endptr | |
6642+
| taint.cpp:729:26:729:32 | ref arg & ... | taint.cpp:732:8:732:13 | endptr | |
6643+
| taint.cpp:729:27:729:32 | endptr | taint.cpp:729:26:729:32 | & ... | |
6644+
| taint.cpp:731:7:731:12 | ref arg endptr | taint.cpp:732:8:732:13 | endptr | |
6645+
| taint.cpp:732:8:732:13 | endptr | taint.cpp:732:7:732:13 | * ... | TAINT |
66326646
| vector.cpp:16:43:16:49 | source1 | vector.cpp:17:26:17:32 | source1 | |
66336647
| vector.cpp:16:43:16:49 | source1 | vector.cpp:31:38:31:44 | source1 | |
66346648
| vector.cpp:17:21:17:33 | call to vector | vector.cpp:19:14:19:14 | v | |

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -720,4 +720,14 @@ void test_strtok_indirect() {
720720
char* tokenized = strtok(source, delim);
721721
sink(*tokenized); // $ ir MISSING: ast
722722
sink(*delim);
723+
}
724+
725+
long int strtol(const char*, char**, int);
726+
727+
void test_strtol(char *source) {
728+
char* endptr = nullptr;
729+
long l = strtol(source, &endptr, 10);
730+
sink(l); // $ ast,ir
731+
sink(endptr); // $ ast,ir
732+
sink(*endptr); // $ ast,ir
723733
}

‎cpp/ql/test/library-tests/ir/ir/PrintAST.expected

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15938,6 +15938,44 @@ ir.cpp:
1593815938
# 2104| Type = [CTypedefType,Size_t] size_t
1593915939
# 2104| ValueCategory = prvalue(load)
1594015940
# 2105| getStmt(6): [ReturnStmt] return ...
15941+
# 2107| [TopLevelFunction] double strtod(char const*, char**)
15942+
# 2107| <params>:
15943+
# 2107| getParameter(0): [Parameter] str
15944+
# 2107| Type = [PointerType] const char *
15945+
# 2107| getParameter(1): [Parameter] endptr
15946+
# 2107| Type = [PointerType] char **
15947+
# 2109| [TopLevelFunction] char* test_strtod(char*)
15948+
# 2109| <params>:
15949+
# 2109| getParameter(0): [Parameter] s
15950+
# 2109| Type = [CharPointerType] char *
15951+
# 2109| getEntryPoint(): [BlockStmt] { ... }
15952+
# 2110| getStmt(0): [DeclStmt] declaration
15953+
# 2110| getDeclarationEntry(0): [VariableDeclarationEntry] definition of end
15954+
# 2110| Type = [CharPointerType] char *
15955+
# 2111| getStmt(1): [DeclStmt] declaration
15956+
# 2111| getDeclarationEntry(0): [VariableDeclarationEntry] definition of d
15957+
# 2111| Type = [DoubleType] double
15958+
# 2111| getVariable().getInitializer(): [Initializer] initializer for d
15959+
# 2111| getExpr(): [FunctionCall] call to strtod
15960+
# 2111| Type = [DoubleType] double
15961+
# 2111| ValueCategory = prvalue
15962+
# 2111| getArgument(0): [VariableAccess] s
15963+
# 2111| Type = [CharPointerType] char *
15964+
# 2111| ValueCategory = prvalue(load)
15965+
# 2111| getArgument(1): [AddressOfExpr] & ...
15966+
# 2111| Type = [PointerType] char **
15967+
# 2111| ValueCategory = prvalue
15968+
# 2111| getOperand(): [VariableAccess] end
15969+
# 2111| Type = [CharPointerType] char *
15970+
# 2111| ValueCategory = lvalue
15971+
# 2111| getArgument(0).getFullyConverted(): [CStyleCast] (const char *)...
15972+
# 2111| Conversion = [PointerConversion] pointer conversion
15973+
# 2111| Type = [PointerType] const char *
15974+
# 2111| ValueCategory = prvalue
15975+
# 2112| getStmt(2): [ReturnStmt] return ...
15976+
# 2112| getExpr(): [VariableAccess] end
15977+
# 2112| Type = [CharPointerType] char *
15978+
# 2112| ValueCategory = prvalue(load)
1594115979
perf-regression.cpp:
1594215980
# 4| [CopyAssignmentOperator] Big& Big::operator=(Big const&)
1594315981
# 4| <params>:

‎cpp/ql/test/library-tests/ir/ir/aliased_ir.expected

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12333,6 +12333,40 @@ ir.cpp:
1233312333
# 2098| v2098_8(void) = AliasedUse : ~m2104_8
1233412334
# 2098| v2098_9(void) = ExitFunction :
1233512335

12336+
# 2109| char* test_strtod(char*)
12337+
# 2109| Block 0
12338+
# 2109| v2109_1(void) = EnterFunction :
12339+
# 2109| m2109_2(unknown) = AliasedDefinition :
12340+
# 2109| m2109_3(unknown) = InitializeNonLocal :
12341+
# 2109| m2109_4(unknown) = Chi : total:m2109_2, partial:m2109_3
12342+
# 2109| r2109_5(glval<char *>) = VariableAddress[s] :
12343+
# 2109| m2109_6(char *) = InitializeParameter[s] : &:r2109_5
12344+
# 2109| r2109_7(char *) = Load[s] : &:r2109_5, m2109_6
12345+
# 2109| m2109_8(unknown) = InitializeIndirection[s] : &:r2109_7
12346+
# 2110| r2110_1(glval<char *>) = VariableAddress[end] :
12347+
# 2110| m2110_2(char *) = Uninitialized[end] : &:r2110_1
12348+
# 2111| r2111_1(glval<double>) = VariableAddress[d] :
12349+
# 2111| r2111_2(glval<unknown>) = FunctionAddress[strtod] :
12350+
# 2111| r2111_3(glval<char *>) = VariableAddress[s] :
12351+
# 2111| r2111_4(char *) = Load[s] : &:r2111_3, m2109_6
12352+
# 2111| r2111_5(char *) = Convert : r2111_4
12353+
# 2111| r2111_6(glval<char *>) = VariableAddress[end] :
12354+
# 2111| r2111_7(char **) = CopyValue : r2111_6
12355+
# 2111| r2111_8(double) = Call[strtod] : func:r2111_2, 0:r2111_5, 1:r2111_7
12356+
# 2111| v2111_9(void) = ^BufferReadSideEffect[0] : &:r2111_5, ~m2109_8
12357+
# 2111| m2111_10(char *) = ^IndirectMayWriteSideEffect[1] : &:r2111_7
12358+
# 2111| m2111_11(char *) = Chi : total:m2110_2, partial:m2111_10
12359+
# 2111| m2111_12(double) = Store[d] : &:r2111_1, r2111_8
12360+
# 2112| r2112_1(glval<char *>) = VariableAddress[#return] :
12361+
# 2112| r2112_2(glval<char *>) = VariableAddress[end] :
12362+
# 2112| r2112_3(char *) = Load[end] : &:r2112_2, m2111_11
12363+
# 2112| m2112_4(char *) = Store[#return] : &:r2112_1, r2112_3
12364+
# 2109| v2109_9(void) = ReturnIndirection[s] : &:r2109_7, m2109_8
12365+
# 2109| r2109_10(glval<char *>) = VariableAddress[#return] :
12366+
# 2109| v2109_11(void) = ReturnValue : &:r2109_10, m2112_4
12367+
# 2109| v2109_12(void) = AliasedUse : m2109_3
12368+
# 2109| v2109_13(void) = ExitFunction :
12369+
1233612370
perf-regression.cpp:
1233712371
# 6| void Big::Big()
1233812372
# 6| Block 0

‎cpp/ql/test/library-tests/ir/ir/ir.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2104,4 +2104,12 @@ void newArrayCorrectType(size_t n) {
21042104
new int[n] { 0, 1, 2 };
21052105
}
21062106

2107+
double strtod (const char* str, char** endptr);
2108+
2109+
char* test_strtod(char *s) {
2110+
char *end;
2111+
double d = strtod(s, &end);
2112+
return end;
2113+
}
2114+
21072115
// semmle-extractor-options: -std=c++17 --clang

‎cpp/ql/test/library-tests/ir/ir/operand_locations.expected

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10004,6 +10004,36 @@
1000410004
| ir.cpp:2104:11:2104:11 | Address | &:r2104_2 |
1000510005
| ir.cpp:2104:11:2104:11 | Left | r2104_3 |
1000610006
| ir.cpp:2104:11:2104:11 | Load | m2098_6 |
10007+
| ir.cpp:2109:7:2109:17 | Address | &:r2109_10 |
10008+
| ir.cpp:2109:7:2109:17 | ChiPartial | partial:m2109_3 |
10009+
| ir.cpp:2109:7:2109:17 | ChiTotal | total:m2109_2 |
10010+
| ir.cpp:2109:7:2109:17 | Load | m2112_4 |
10011+
| ir.cpp:2109:7:2109:17 | SideEffect | m2109_3 |
10012+
| ir.cpp:2109:25:2109:25 | Address | &:r2109_5 |
10013+
| ir.cpp:2109:25:2109:25 | Address | &:r2109_5 |
10014+
| ir.cpp:2109:25:2109:25 | Address | &:r2109_7 |
10015+
| ir.cpp:2109:25:2109:25 | Address | &:r2109_7 |
10016+
| ir.cpp:2109:25:2109:25 | Load | m2109_6 |
10017+
| ir.cpp:2109:25:2109:25 | SideEffect | m2109_8 |
10018+
| ir.cpp:2110:9:2110:11 | Address | &:r2110_1 |
10019+
| ir.cpp:2111:10:2111:10 | Address | &:r2111_1 |
10020+
| ir.cpp:2111:14:2111:19 | CallTarget | func:r2111_2 |
10021+
| ir.cpp:2111:14:2111:19 | StoreValue | r2111_8 |
10022+
| ir.cpp:2111:21:2111:21 | Address | &:r2111_3 |
10023+
| ir.cpp:2111:21:2111:21 | Address | &:r2111_5 |
10024+
| ir.cpp:2111:21:2111:21 | Arg(0) | 0:r2111_5 |
10025+
| ir.cpp:2111:21:2111:21 | Load | m2109_6 |
10026+
| ir.cpp:2111:21:2111:21 | SideEffect | ~m2109_8 |
10027+
| ir.cpp:2111:21:2111:21 | Unary | r2111_4 |
10028+
| ir.cpp:2111:24:2111:27 | Address | &:r2111_7 |
10029+
| ir.cpp:2111:24:2111:27 | Arg(1) | 1:r2111_7 |
10030+
| ir.cpp:2111:24:2111:27 | ChiPartial | partial:m2111_10 |
10031+
| ir.cpp:2111:24:2111:27 | ChiTotal | total:m2110_2 |
10032+
| ir.cpp:2111:25:2111:27 | Unary | r2111_6 |
10033+
| ir.cpp:2112:3:2112:13 | Address | &:r2112_1 |
10034+
| ir.cpp:2112:10:2112:12 | Address | &:r2112_2 |
10035+
| ir.cpp:2112:10:2112:12 | Load | m2111_11 |
10036+
| ir.cpp:2112:10:2112:12 | StoreValue | r2112_3 |
1000710037
| perf-regression.cpp:6:3:6:5 | Address | &:r6_5 |
1000810038
| perf-regression.cpp:6:3:6:5 | Address | &:r6_5 |
1000910039
| perf-regression.cpp:6:3:6:5 | Address | &:r6_7 |

‎cpp/ql/test/library-tests/ir/ir/raw_ir.expected

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11538,6 +11538,38 @@ ir.cpp:
1153811538
# 2098| v2098_7(void) = AliasedUse : ~m?
1153911539
# 2098| v2098_8(void) = ExitFunction :
1154011540

11541+
# 2109| char* test_strtod(char*)
11542+
# 2109| Block 0
11543+
# 2109| v2109_1(void) = EnterFunction :
11544+
# 2109| mu2109_2(unknown) = AliasedDefinition :
11545+
# 2109| mu2109_3(unknown) = InitializeNonLocal :
11546+
# 2109| r2109_4(glval<char *>) = VariableAddress[s] :
11547+
# 2109| mu2109_5(char *) = InitializeParameter[s] : &:r2109_4
11548+
# 2109| r2109_6(char *) = Load[s] : &:r2109_4, ~m?
11549+
# 2109| mu2109_7(unknown) = InitializeIndirection[s] : &:r2109_6
11550+
# 2110| r2110_1(glval<char *>) = VariableAddress[end] :
11551+
# 2110| mu2110_2(char *) = Uninitialized[end] : &:r2110_1
11552+
# 2111| r2111_1(glval<double>) = VariableAddress[d] :
11553+
# 2111| r2111_2(glval<unknown>) = FunctionAddress[strtod] :
11554+
# 2111| r2111_3(glval<char *>) = VariableAddress[s] :
11555+
# 2111| r2111_4(char *) = Load[s] : &:r2111_3, ~m?
11556+
# 2111| r2111_5(char *) = Convert : r2111_4
11557+
# 2111| r2111_6(glval<char *>) = VariableAddress[end] :
11558+
# 2111| r2111_7(char **) = CopyValue : r2111_6
11559+
# 2111| r2111_8(double) = Call[strtod] : func:r2111_2, 0:r2111_5, 1:r2111_7
11560+
# 2111| v2111_9(void) = ^BufferReadSideEffect[0] : &:r2111_5, ~m?
11561+
# 2111| mu2111_10(char *) = ^IndirectMayWriteSideEffect[1] : &:r2111_7
11562+
# 2111| mu2111_11(double) = Store[d] : &:r2111_1, r2111_8
11563+
# 2112| r2112_1(glval<char *>) = VariableAddress[#return] :
11564+
# 2112| r2112_2(glval<char *>) = VariableAddress[end] :
11565+
# 2112| r2112_3(char *) = Load[end] : &:r2112_2, ~m?
11566+
# 2112| mu2112_4(char *) = Store[#return] : &:r2112_1, r2112_3
11567+
# 2109| v2109_8(void) = ReturnIndirection[s] : &:r2109_6, ~m?
11568+
# 2109| r2109_9(glval<char *>) = VariableAddress[#return] :
11569+
# 2109| v2109_10(void) = ReturnValue : &:r2109_9, ~m?
11570+
# 2109| v2109_11(void) = AliasedUse : ~m?
11571+
# 2109| v2109_12(void) = ExitFunction :
11572+
1154111573
perf-regression.cpp:
1154211574
# 6| void Big::Big()
1154311575
# 6| Block 0

0 commit comments

Comments
 (0)