Skip to content

Commit 9f50f67

Browse files
geoffw0geoffw0
authored
Merge pull request github#5065 from MathiasVP/scanf-model
C++: Add sscanf and fscanf models
2 parents 4fdbda3 + 0db54e0 commit 9f50f67

File tree

5 files changed

+88
-3
lines changed

5 files changed

+88
-3
lines changed

‎cpp/ql/src/semmle/code/cpp/commons/Scanf.qll

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,9 @@ class Snscanf extends ScanfFunction {
9292
this instanceof TopLevelFunction and
9393
(
9494
hasName("_snscanf") or // _snscanf(src, max_amount, format, args...)
95-
hasName("_snwscanf") // _snwscanf(src, max_amount, format, args...)
95+
hasName("_snwscanf") or // _snwscanf(src, max_amount, format, args...)
96+
hasName("_snscanf_l") or // _snscanf_l(src, max_amount, format, locale, args...)
97+
hasName("_snwscanf_l") // _snwscanf_l(src, max_amount, format, locale, args...)
9698
// note that the max_amount is not a limit on the output length, it's an input length
9799
// limit used with non null-terminated strings.
98100
)
@@ -101,6 +103,12 @@ class Snscanf extends ScanfFunction {
101103
override int getInputParameterIndex() { result = 0 }
102104

103105
override int getFormatParameterIndex() { result = 2 }
106+
107+
/**
108+
* Gets the position at which the maximum number of characters in the
109+
* input string is specified.
110+
*/
111+
int getInputLengthParameterIndex() { result = 1 }
104112
}
105113

106114
/**

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,4 @@ private import implementations.StdString
2525
private import implementations.Swap
2626
private import implementations.GetDelim
2727
private import implementations.SmartPointer
28+
private import implementations.Sscanf

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

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/**
2+
* Provides implementation classes modeling `sscanf`, `fscanf` and various similar
3+
* functions. See `semmle.code.cpp.models.Models` for usage information.
4+
*/
5+
6+
import semmle.code.cpp.Function
7+
import semmle.code.cpp.commons.Scanf
8+
import semmle.code.cpp.models.interfaces.ArrayFunction
9+
import semmle.code.cpp.models.interfaces.Taint
10+
import semmle.code.cpp.models.interfaces.Alias
11+
import semmle.code.cpp.models.interfaces.SideEffect
12+
13+
/**
14+
* The standard function `sscanf`, `fscanf` and its assorted variants
15+
*/
16+
private class SscanfModel extends ArrayFunction, TaintFunction, AliasFunction, SideEffectFunction {
17+
SscanfModel() { this instanceof Sscanf or this instanceof Fscanf or this instanceof Snscanf }
18+
19+
override predicate hasArrayWithNullTerminator(int bufParam) {
20+
bufParam = this.(ScanfFunction).getFormatParameterIndex()
21+
or
22+
not this instanceof Fscanf and
23+
bufParam = this.(ScanfFunction).getInputParameterIndex()
24+
}
25+
26+
override predicate hasArrayInput(int bufParam) { hasArrayWithNullTerminator(bufParam) }
27+
28+
private int getLengthParameterIndex() { result = this.(Snscanf).getInputLengthParameterIndex() }
29+
30+
private int getLocaleParameterIndex() {
31+
this.getName().matches("%\\_l") and
32+
(
33+
if exists(getLengthParameterIndex())
34+
then result = getLengthParameterIndex() + 2
35+
else result = 2
36+
)
37+
}
38+
39+
private int getArgsStartPosition() { result = this.getNumberOfParameters() }
40+
41+
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
42+
input.isParameterDeref(this.(ScanfFunction).getInputParameterIndex()) and
43+
output.isParameterDeref(any(int i | i >= getArgsStartPosition()))
44+
}
45+
46+
override predicate parameterNeverEscapes(int index) {
47+
index = [0 .. max(getACallToThisFunction().getNumberOfArguments())]
48+
}
49+
50+
override predicate parameterEscapesOnlyViaReturn(int index) { none() }
51+
52+
override predicate parameterIsAlwaysReturned(int index) { none() }
53+
54+
override predicate hasOnlySpecificReadSideEffects() { any() }
55+
56+
override predicate hasOnlySpecificWriteSideEffects() { any() }
57+
58+
override predicate hasSpecificWriteSideEffect(ParameterIndex i, boolean buffer, boolean mustWrite) {
59+
i >= getArgsStartPosition() and
60+
buffer = true and
61+
mustWrite = true
62+
}
63+
64+
override predicate hasSpecificReadSideEffect(ParameterIndex i, boolean buffer) {
65+
buffer = true and
66+
i =
67+
[
68+
this.(ScanfFunction).getInputParameterIndex(),
69+
this.(ScanfFunction).getFormatParameterIndex(), getLocaleParameterIndex()
70+
]
71+
}
72+
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ void test1()
123123
{
124124
int i = 0;
125125
sink(sscanf(string::source(), "%i", &i));
126-
sink(i); // $ MISSING: ast,ir
126+
sink(i); // $ ast,ir
127127
}
128128
{
129129
char buffer[256] = {0};
@@ -133,7 +133,7 @@ void test1()
133133
{
134134
char buffer[256] = {0};
135135
sink(sscanf(string::source(), "%s", &buffer));
136-
sink(buffer); // $ MISSING: ast,ir
136+
sink(buffer); // $ ast,ir
137137
}
138138
}
139139

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,23 +378,27 @@
378378
| format.cpp:114:37:114:50 | call to source | format.cpp:114:18:114:23 | ref arg buffer | TAINT |
379379
| format.cpp:119:10:119:11 | 0 | format.cpp:120:29:120:29 | i | |
380380
| format.cpp:119:10:119:11 | 0 | format.cpp:121:8:121:8 | i | |
381+
| format.cpp:120:15:120:19 | 123 | format.cpp:120:28:120:29 | ref arg & ... | TAINT |
381382
| format.cpp:120:28:120:29 | ref arg & ... | format.cpp:120:29:120:29 | i [inner post update] | |
382383
| format.cpp:120:28:120:29 | ref arg & ... | format.cpp:121:8:121:8 | i | |
383384
| format.cpp:120:29:120:29 | i | format.cpp:120:28:120:29 | & ... | |
384385
| format.cpp:124:10:124:11 | 0 | format.cpp:125:40:125:40 | i | |
385386
| format.cpp:124:10:124:11 | 0 | format.cpp:126:8:126:8 | i | |
387+
| format.cpp:125:15:125:28 | call to source | format.cpp:125:39:125:40 | ref arg & ... | TAINT |
386388
| format.cpp:125:39:125:40 | ref arg & ... | format.cpp:125:40:125:40 | i [inner post update] | |
387389
| format.cpp:125:39:125:40 | ref arg & ... | format.cpp:126:8:126:8 | i | |
388390
| format.cpp:125:40:125:40 | i | format.cpp:125:39:125:40 | & ... | |
389391
| format.cpp:129:21:129:24 | {...} | format.cpp:130:32:130:37 | buffer | |
390392
| format.cpp:129:21:129:24 | {...} | format.cpp:131:8:131:13 | buffer | |
391393
| format.cpp:129:23:129:23 | 0 | format.cpp:129:21:129:24 | {...} | TAINT |
394+
| format.cpp:130:15:130:22 | Hello. | format.cpp:130:31:130:37 | ref arg & ... | TAINT |
392395
| format.cpp:130:31:130:37 | ref arg & ... | format.cpp:130:32:130:37 | buffer [inner post update] | |
393396
| format.cpp:130:31:130:37 | ref arg & ... | format.cpp:131:8:131:13 | buffer | |
394397
| format.cpp:130:32:130:37 | buffer | format.cpp:130:31:130:37 | & ... | |
395398
| format.cpp:134:21:134:24 | {...} | format.cpp:135:40:135:45 | buffer | |
396399
| format.cpp:134:21:134:24 | {...} | format.cpp:136:8:136:13 | buffer | |
397400
| format.cpp:134:23:134:23 | 0 | format.cpp:134:21:134:24 | {...} | TAINT |
401+
| format.cpp:135:15:135:28 | call to source | format.cpp:135:39:135:45 | ref arg & ... | TAINT |
398402
| format.cpp:135:39:135:45 | ref arg & ... | format.cpp:135:40:135:45 | buffer [inner post update] | |
399403
| format.cpp:135:39:135:45 | ref arg & ... | format.cpp:136:8:136:13 | buffer | |
400404
| format.cpp:135:40:135:45 | buffer | format.cpp:135:39:135:45 | & ... | |

0 commit comments

Comments
 (0)