Skip to content

Commit 89cd479

Browse files
jketemajketema
authored
Merge pull request github#11610 from jketema/scanf
C++: Model `scanf` and `fscanf` as flow sources
2 parents f373b7f + 8f9a73e commit 89cd479

File tree

8 files changed

+102
-60
lines changed

8 files changed

+102
-60
lines changed

‎cpp/ql/lib/change-notes/2022-12-08-support-scanf-dataflow.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The `scanf` and `fscanf` functions and their variants are now recognized as flow sources.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ private import implementations.StdString
2727
private import implementations.Swap
2828
private import implementations.GetDelim
2929
private import implementations.SmartPointer
30-
private import implementations.Sscanf
30+
private import implementations.Scanf
3131
private import implementations.Send
3232
private import implementations.Recv
3333
private import implementations.Accept

‎cpp/ql/lib/semmle/code/cpp/models/implementations/Sscanf.qll renamed to ‎cpp/ql/lib/semmle/code/cpp/models/implementations/Scanf.qll

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
2-
* Provides implementation classes modeling `sscanf`, `fscanf` and various similar
3-
* functions. See `semmle.code.cpp.models.Models` for usage information.
2+
* Provides implementation classes modeling the `scanf` family of functions.
3+
* See `semmle.code.cpp.models.Models` for usage information.
44
*/
55

66
import semmle.code.cpp.Function
@@ -9,18 +9,15 @@ import semmle.code.cpp.models.interfaces.ArrayFunction
99
import semmle.code.cpp.models.interfaces.Taint
1010
import semmle.code.cpp.models.interfaces.Alias
1111
import semmle.code.cpp.models.interfaces.SideEffect
12+
import semmle.code.cpp.models.interfaces.FlowSource
1213

1314
/**
14-
* The standard function `sscanf`, `fscanf` and its assorted variants
15+
* The `scanf` family of functions.
1516
*/
16-
private class SscanfModel extends ArrayFunction, TaintFunction, AliasFunction, SideEffectFunction {
17-
SscanfModel() { this instanceof Sscanf or this instanceof Fscanf or this instanceof Snscanf }
18-
17+
abstract private class ScanfFunctionModel extends ArrayFunction, TaintFunction, AliasFunction,
18+
SideEffectFunction {
1919
override predicate hasArrayWithNullTerminator(int bufParam) {
2020
bufParam = this.(ScanfFunction).getFormatParameterIndex()
21-
or
22-
not this instanceof Fscanf and
23-
bufParam = this.(ScanfFunction).getInputParameterIndex()
2421
}
2522

2623
override predicate hasArrayInput(int bufParam) { this.hasArrayWithNullTerminator(bufParam) }
@@ -36,7 +33,7 @@ private class SscanfModel extends ArrayFunction, TaintFunction, AliasFunction, S
3633
)
3734
}
3835

39-
private int getArgsStartPosition() { result = this.getNumberOfParameters() }
36+
int getArgsStartPosition() { result = this.getNumberOfParameters() }
4037

4138
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
4239
input.isParameterDeref(this.(ScanfFunction).getInputParameterIndex()) and
@@ -70,3 +67,36 @@ private class SscanfModel extends ArrayFunction, TaintFunction, AliasFunction, S
7067
]
7168
}
7269
}
70+
71+
/**
72+
* The standard function `scanf` and its assorted variants
73+
*/
74+
private class ScanfModel extends ScanfFunctionModel, LocalFlowSourceFunction instanceof Scanf {
75+
override predicate hasLocalFlowSource(FunctionOutput output, string description) {
76+
output.isParameterDeref(any(int i | i >= this.getArgsStartPosition())) and
77+
description = "Value read by " + this.getName()
78+
}
79+
}
80+
81+
/**
82+
* The standard function `fscanf` and its assorted variants
83+
*/
84+
private class FscanfModel extends ScanfFunctionModel, RemoteFlowSourceFunction instanceof Fscanf {
85+
override predicate hasRemoteFlowSource(FunctionOutput output, string description) {
86+
output.isParameterDeref(any(int i | i >= this.getArgsStartPosition())) and
87+
description = "Value read by " + this.getName()
88+
}
89+
}
90+
91+
/**
92+
* The standard function `sscanf` and its assorted variants
93+
*/
94+
private class SscanfModel extends ScanfFunctionModel {
95+
SscanfModel() { this instanceof Sscanf or this instanceof Snscanf }
96+
97+
override predicate hasArrayWithNullTerminator(int bufParam) {
98+
super.hasArrayWithNullTerminator(bufParam)
99+
or
100+
bufParam = this.(ScanfFunction).getInputParameterIndex()
101+
}
102+
}

‎cpp/ql/test/library-tests/dataflow/source-sink-tests/local-flow.ql

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,20 @@ class LocalFlowSourceTest extends InlineExpectationsTest {
1111

1212
override predicate hasActualResult(Location location, string element, string tag, string value) {
1313
tag = "local_source" and
14-
value = "" and
15-
exists(LocalFlowSource node |
14+
exists(LocalFlowSource node, int n |
15+
n =
16+
strictcount(LocalFlowSource otherNode |
17+
node.getLocation().getStartLine() = otherNode.getLocation().getStartLine()
18+
) and
19+
(
20+
n = 1 and value = ""
21+
or
22+
// If there is more than one node on this line
23+
// we specify the location explicitly.
24+
n > 1 and
25+
value =
26+
node.getLocation().getStartLine().toString() + ":" + node.getLocation().getStartColumn()
27+
) and
1628
location = node.getLocation() and
1729
element = node.toString()
1830
)

‎cpp/ql/test/library-tests/dataflow/source-sink-tests/remote-flow.ql

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,20 @@ class RemoteFlowSourceTest extends InlineExpectationsTest {
1111

1212
override predicate hasActualResult(Location location, string element, string tag, string value) {
1313
tag = "remote_source" and
14-
value = "" and
15-
exists(RemoteFlowSource node |
14+
exists(RemoteFlowSource node, int n |
15+
n =
16+
strictcount(RemoteFlowSource otherNode |
17+
node.getLocation().getStartLine() = otherNode.getLocation().getStartLine()
18+
) and
19+
(
20+
n = 1 and value = ""
21+
or
22+
// If there is more than one node on this line
23+
// we specify the location explicitly.
24+
n > 1 and
25+
value =
26+
node.getLocation().getStartLine().toString() + ":" + node.getLocation().getStartColumn()
27+
) and
1628
location = node.getLocation() and
1729
element = node.toString()
1830
)
@@ -26,8 +38,20 @@ class RemoteFlowSinkTest extends InlineExpectationsTest {
2638

2739
override predicate hasActualResult(Location location, string element, string tag, string value) {
2840
tag = "remote_sink" and
29-
value = "" and
30-
exists(RemoteFlowSink node |
41+
exists(RemoteFlowSink node, int n |
42+
n =
43+
strictcount(RemoteFlowSink otherNode |
44+
node.getLocation().getStartLine() = otherNode.getLocation().getStartLine()
45+
) and
46+
(
47+
n = 1 and value = ""
48+
or
49+
// If there is more than one node on this line
50+
// we specify the location explicitly.
51+
n > 1 and
52+
value =
53+
node.getLocation().getStartLine().toString() + ":" + node.getLocation().getStartColumn()
54+
) and
3155
location = node.getLocation() and
3256
element = node.toString()
3357
)

‎cpp/ql/test/library-tests/dataflow/source-sink-tests/sources-and-sinks.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,17 @@ void test_readv_and_writev(iovec* iovs) {
2626
readv(0, iovs, 16); // $ remote_source
2727
writev(0, iovs, 16); // $ remote_sink
2828
}
29+
30+
struct FILE;
31+
32+
int fscanf(FILE *stream, const char *format, ...);
33+
int scanf(const char *format, ...);
34+
35+
void test_scanf(FILE *stream, int *d, char *buf) {
36+
scanf(""); // Not a local source, as there are no output arguments
37+
fscanf(stream, ""); // Not a remote source, as there are no output arguments
38+
scanf("%d", d); // $ local_source
39+
fscanf(stream, "%d", d); // $ remote_source
40+
scanf("%d %s", d, buf); // $ local_source=40:18 local_source=40:21
41+
fscanf(stream, "%d %s", d, buf); // $ remote_source=41:27 remote_source=41:30
42+
}

‎cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/TaintedPath.expected

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
edges
22
| test.c:9:23:9:26 | argv | test.c:17:11:17:18 | fileName indirection |
33
| test.c:31:22:31:25 | argv | test.c:32:11:32:18 | fileName indirection |
4-
| test.c:37:17:37:24 | fileName | test.c:38:11:38:18 | fileName indirection |
54
| test.c:37:17:37:24 | scanf output argument | test.c:38:11:38:18 | fileName indirection |
65
| test.c:43:17:43:24 | fileName | test.c:44:11:44:18 | fileName indirection |
76
| test.c:43:17:43:24 | scanf output argument | test.c:44:11:44:18 | fileName indirection |
@@ -10,7 +9,6 @@ nodes
109
| test.c:17:11:17:18 | fileName indirection | semmle.label | fileName indirection |
1110
| test.c:31:22:31:25 | argv | semmle.label | argv |
1211
| test.c:32:11:32:18 | fileName indirection | semmle.label | fileName indirection |
13-
| test.c:37:17:37:24 | fileName | semmle.label | fileName |
1412
| test.c:37:17:37:24 | scanf output argument | semmle.label | scanf output argument |
1513
| test.c:38:11:38:18 | fileName indirection | semmle.label | fileName indirection |
1614
| test.c:43:17:43:24 | fileName | semmle.label | fileName |
@@ -20,5 +18,5 @@ subpaths
2018
#select
2119
| test.c:17:11:17:18 | fileName | test.c:9:23:9:26 | argv | test.c:17:11:17:18 | fileName indirection | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:9:23:9:26 | argv | user input (argv) |
2220
| test.c:32:11:32:18 | fileName | test.c:31:22:31:25 | argv | test.c:32:11:32:18 | fileName indirection | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:31:22:31:25 | argv | user input (argv) |
23-
| test.c:38:11:38:18 | fileName | test.c:37:17:37:24 | fileName | test.c:38:11:38:18 | fileName indirection | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:37:17:37:24 | fileName | user input (scanf) |
21+
| test.c:38:11:38:18 | fileName | test.c:37:17:37:24 | scanf output argument | test.c:38:11:38:18 | fileName indirection | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:37:17:37:24 | fileName | user input (scanf) |
2422
| test.c:44:11:44:18 | fileName | test.c:43:17:43:24 | fileName | test.c:44:11:44:18 | fileName indirection | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:43:17:43:24 | fileName | user input (scanf) |

‎cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/UnboundedWrite.expected

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -7,42 +7,12 @@ edges
77
| tests.c:28:22:28:25 | argv | tests.c:28:22:28:28 | access to array |
88
| tests.c:28:22:28:25 | argv | tests.c:28:22:28:28 | access to array indirection |
99
| tests.c:28:22:28:25 | argv | tests.c:28:22:28:28 | access to array indirection |
10-
| tests.c:28:22:28:25 | argv | tests.c:31:15:31:23 | buffer100 |
11-
| tests.c:28:22:28:25 | argv | tests.c:31:15:31:23 | buffer100 |
12-
| tests.c:28:22:28:25 | argv | tests.c:31:15:31:23 | buffer100 indirection |
13-
| tests.c:28:22:28:25 | argv | tests.c:31:15:31:23 | buffer100 indirection |
14-
| tests.c:28:22:28:25 | argv | tests.c:33:21:33:29 | buffer100 |
15-
| tests.c:28:22:28:25 | argv | tests.c:33:21:33:29 | buffer100 |
16-
| tests.c:28:22:28:25 | argv | tests.c:33:21:33:29 | buffer100 indirection |
17-
| tests.c:28:22:28:25 | argv | tests.c:33:21:33:29 | buffer100 indirection |
1810
| tests.c:29:28:29:31 | argv | tests.c:29:28:29:34 | access to array |
1911
| tests.c:29:28:29:31 | argv | tests.c:29:28:29:34 | access to array |
2012
| tests.c:29:28:29:31 | argv | tests.c:29:28:29:34 | access to array |
2113
| tests.c:29:28:29:31 | argv | tests.c:29:28:29:34 | access to array |
2214
| tests.c:29:28:29:31 | argv | tests.c:29:28:29:34 | access to array indirection |
2315
| tests.c:29:28:29:31 | argv | tests.c:29:28:29:34 | access to array indirection |
24-
| tests.c:29:28:29:31 | argv | tests.c:31:15:31:23 | buffer100 |
25-
| tests.c:29:28:29:31 | argv | tests.c:31:15:31:23 | buffer100 |
26-
| tests.c:29:28:29:31 | argv | tests.c:31:15:31:23 | buffer100 indirection |
27-
| tests.c:29:28:29:31 | argv | tests.c:31:15:31:23 | buffer100 indirection |
28-
| tests.c:29:28:29:31 | argv | tests.c:33:21:33:29 | buffer100 |
29-
| tests.c:29:28:29:31 | argv | tests.c:33:21:33:29 | buffer100 |
30-
| tests.c:29:28:29:31 | argv | tests.c:33:21:33:29 | buffer100 indirection |
31-
| tests.c:29:28:29:31 | argv | tests.c:33:21:33:29 | buffer100 indirection |
32-
| tests.c:31:15:31:23 | array to pointer conversion | tests.c:31:15:31:23 | buffer100 |
33-
| tests.c:31:15:31:23 | array to pointer conversion | tests.c:31:15:31:23 | buffer100 indirection |
34-
| tests.c:31:15:31:23 | array to pointer conversion | tests.c:33:21:33:29 | buffer100 |
35-
| tests.c:31:15:31:23 | array to pointer conversion | tests.c:33:21:33:29 | buffer100 indirection |
36-
| tests.c:31:15:31:23 | buffer100 | tests.c:31:15:31:23 | buffer100 |
37-
| tests.c:31:15:31:23 | buffer100 | tests.c:31:15:31:23 | buffer100 indirection |
38-
| tests.c:31:15:31:23 | buffer100 | tests.c:33:21:33:29 | buffer100 |
39-
| tests.c:31:15:31:23 | buffer100 | tests.c:33:21:33:29 | buffer100 indirection |
40-
| tests.c:31:15:31:23 | scanf output argument | tests.c:33:21:33:29 | buffer100 |
41-
| tests.c:31:15:31:23 | scanf output argument | tests.c:33:21:33:29 | buffer100 indirection |
42-
| tests.c:33:21:33:29 | array to pointer conversion | tests.c:33:21:33:29 | buffer100 |
43-
| tests.c:33:21:33:29 | array to pointer conversion | tests.c:33:21:33:29 | buffer100 indirection |
44-
| tests.c:33:21:33:29 | buffer100 | tests.c:33:21:33:29 | buffer100 |
45-
| tests.c:33:21:33:29 | buffer100 | tests.c:33:21:33:29 | buffer100 indirection |
4616
| tests.c:34:10:34:13 | argv | tests.c:34:10:34:16 | (const char *)... |
4717
| tests.c:34:10:34:13 | argv | tests.c:34:10:34:16 | (const char *)... |
4818
| tests.c:34:10:34:13 | argv | tests.c:34:10:34:16 | access to array |
@@ -65,16 +35,11 @@ nodes
6535
| tests.c:29:28:29:34 | access to array | semmle.label | access to array |
6636
| tests.c:29:28:29:34 | access to array indirection | semmle.label | access to array indirection |
6737
| tests.c:31:15:31:23 | array to pointer conversion | semmle.label | array to pointer conversion |
68-
| tests.c:31:15:31:23 | array to pointer conversion | semmle.label | array to pointer conversion |
6938
| tests.c:31:15:31:23 | buffer100 | semmle.label | buffer100 |
7039
| tests.c:31:15:31:23 | buffer100 | semmle.label | buffer100 |
71-
| tests.c:31:15:31:23 | buffer100 indirection | semmle.label | buffer100 indirection |
72-
| tests.c:31:15:31:23 | scanf output argument | semmle.label | scanf output argument |
73-
| tests.c:33:21:33:29 | array to pointer conversion | semmle.label | array to pointer conversion |
7440
| tests.c:33:21:33:29 | array to pointer conversion | semmle.label | array to pointer conversion |
7541
| tests.c:33:21:33:29 | buffer100 | semmle.label | buffer100 |
7642
| tests.c:33:21:33:29 | buffer100 | semmle.label | buffer100 |
77-
| tests.c:33:21:33:29 | buffer100 indirection | semmle.label | buffer100 indirection |
7843
| tests.c:34:10:34:13 | argv | semmle.label | argv |
7944
| tests.c:34:10:34:13 | argv | semmle.label | argv |
8045
| tests.c:34:10:34:16 | (const char *)... | semmle.label | (const char *)... |
@@ -84,11 +49,6 @@ nodes
8449
#select
8550
| tests.c:28:3:28:9 | call to sprintf | tests.c:28:22:28:25 | argv | tests.c:28:22:28:28 | access to array | This 'call to sprintf' with input from $@ may overflow the destination. | tests.c:28:22:28:25 | argv | argv |
8651
| tests.c:29:3:29:9 | call to sprintf | tests.c:29:28:29:31 | argv | tests.c:29:28:29:34 | access to array | This 'call to sprintf' with input from $@ may overflow the destination. | tests.c:29:28:29:31 | argv | argv |
87-
| tests.c:31:15:31:23 | buffer100 | tests.c:28:22:28:25 | argv | tests.c:31:15:31:23 | buffer100 | This 'scanf string argument' with input from $@ may overflow the destination. | tests.c:28:22:28:25 | argv | argv |
88-
| tests.c:31:15:31:23 | buffer100 | tests.c:29:28:29:31 | argv | tests.c:31:15:31:23 | buffer100 | This 'scanf string argument' with input from $@ may overflow the destination. | tests.c:29:28:29:31 | argv | argv |
8952
| tests.c:31:15:31:23 | buffer100 | tests.c:31:15:31:23 | buffer100 | tests.c:31:15:31:23 | buffer100 | This 'scanf string argument' with input from $@ may overflow the destination. | tests.c:31:15:31:23 | buffer100 | buffer100 |
90-
| tests.c:33:21:33:29 | buffer100 | tests.c:28:22:28:25 | argv | tests.c:33:21:33:29 | buffer100 | This 'scanf string argument' with input from $@ may overflow the destination. | tests.c:28:22:28:25 | argv | argv |
91-
| tests.c:33:21:33:29 | buffer100 | tests.c:29:28:29:31 | argv | tests.c:33:21:33:29 | buffer100 | This 'scanf string argument' with input from $@ may overflow the destination. | tests.c:29:28:29:31 | argv | argv |
92-
| tests.c:33:21:33:29 | buffer100 | tests.c:31:15:31:23 | buffer100 | tests.c:33:21:33:29 | buffer100 | This 'scanf string argument' with input from $@ may overflow the destination. | tests.c:31:15:31:23 | buffer100 | buffer100 |
9353
| tests.c:33:21:33:29 | buffer100 | tests.c:33:21:33:29 | buffer100 | tests.c:33:21:33:29 | buffer100 | This 'scanf string argument' with input from $@ may overflow the destination. | tests.c:33:21:33:29 | buffer100 | buffer100 |
9454
| tests.c:34:25:34:33 | buffer100 | tests.c:34:10:34:13 | argv | tests.c:34:10:34:16 | access to array | This 'sscanf string argument' with input from $@ may overflow the destination. | tests.c:34:10:34:13 | argv | argv |

0 commit comments

Comments
 (0)