Skip to content

Commit 14aff5c

Browse files
MathiasVPMathiasVP
committed
C++: Convert 'cpp/missing-check-scanf' to a path-problem query.
1 parent 0fe3072 commit 14aff5c

File tree

2 files changed

+155
-28
lines changed

2 files changed

+155
-28
lines changed

‎cpp/ql/src/Critical/MissingCheckScanf.ql

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* @name Missing return-value check for a 'scanf'-like function
33
* @description Failing to check that a call to 'scanf' actually writes to an
44
* output variable can lead to unexpected behavior at reading time.
5-
* @kind problem
5+
* @kind path-problem
66
* @problem.severity warning
77
* @security-severity 7.5
88
* @precision medium
@@ -20,6 +20,7 @@ import semmle.code.cpp.dataflow.new.DataFlow::DataFlow
2020
import semmle.code.cpp.ir.IR
2121
import semmle.code.cpp.ir.ValueNumbering
2222
import ScanfChecks
23+
import ScanfToUseFlow::PathGraph
2324

2425
/**
2526
* Holds if `n` represents an uninitialized stack-allocated variable, or a
@@ -118,10 +119,13 @@ module ScanfToUseFlow = Global<ScanfToUseConfig>;
118119
* Holds if `source` is the `index`'th argument to the `scanf`-like call `call`, and `sink` is
119120
* a dataflow node that represents the expression `e`.
120121
*/
121-
predicate hasFlow(Node source, ScanfFunctionCall call, int index, Node sink, Expr e) {
122-
isSource(call, index, source, _) and
123-
ScanfToUseFlow::flow(source, sink) and
124-
isSink(sink, e)
122+
predicate flowPath(
123+
ScanfToUseFlow::PathNode source, ScanfFunctionCall call, int index, ScanfToUseFlow::PathNode sink,
124+
Expr e
125+
) {
126+
isSource(call, index, source.getNode(), _) and
127+
ScanfToUseFlow::flowPath(source, sink) and
128+
isSink(sink.getNode(), e)
125129
}
126130

127131
/**
@@ -143,9 +147,12 @@ int getMinimumGuardConstant(ScanfFunctionCall call, int index) {
143147
* Holds the access to `e` isn't guarded by a check that ensures that `call` returned
144148
* at least `minGuard`.
145149
*/
146-
predicate hasNonGuardedAccess(ScanfFunctionCall call, Expr e, int minGuard) {
150+
predicate hasNonGuardedAccess(
151+
ScanfToUseFlow::PathNode source, ScanfFunctionCall call, ScanfToUseFlow::PathNode sink, Expr e,
152+
int minGuard
153+
) {
147154
exists(int index |
148-
hasFlow(_, call, index, _, e) and
155+
flowPath(source, call, index, sink, e) and
149156
minGuard = getMinimumGuardConstant(call, index)
150157
|
151158
not exists(int value |
@@ -173,9 +180,11 @@ BasicBlock blockGuardedBy(int value, string op, ScanfFunctionCall call) {
173180
)
174181
}
175182

176-
from ScanfFunctionCall call, Expr e, int minGuard
177-
where hasNonGuardedAccess(call, e, minGuard)
178-
select e,
183+
from
184+
ScanfToUseFlow::PathNode source, ScanfToUseFlow::PathNode sink, ScanfFunctionCall call, Expr e,
185+
int minGuard
186+
where hasNonGuardedAccess(source, call, sink, e, minGuard)
187+
select e, source, sink,
179188
"This variable is read, but may not have been written. " +
180189
"It should be guarded by a check that the $@ returns at least " + minGuard + ".", call,
181190
call.toString()

‎cpp/ql/test/query-tests/Critical/MissingCheckScanf/MissingCheckScanf.expected

Lines changed: 136 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,136 @@
1-
| test.cpp:35:7:35:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:34:3:34:7 | call to scanf | call to scanf |
2-
| test.cpp:68:7:68:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:67:3:67:7 | call to scanf | call to scanf |
3-
| test.cpp:80:7:80:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:79:3:79:7 | call to scanf | call to scanf |
4-
| test.cpp:90:7:90:8 | * ... | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:89:3:89:7 | call to scanf | call to scanf |
5-
| test.cpp:98:7:98:8 | * ... | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:97:3:97:7 | call to scanf | call to scanf |
6-
| test.cpp:108:7:108:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:107:3:107:8 | call to fscanf | call to fscanf |
7-
| test.cpp:115:7:115:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:114:3:114:8 | call to sscanf | call to sscanf |
8-
| test.cpp:224:8:224:8 | j | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 2. | test.cpp:221:7:221:11 | call to scanf | call to scanf |
9-
| test.cpp:248:9:248:9 | d | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 2. | test.cpp:246:25:246:29 | call to scanf | call to scanf |
10-
| test.cpp:252:9:252:9 | d | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 2. | test.cpp:250:14:250:18 | call to scanf | call to scanf |
11-
| test.cpp:272:7:272:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:271:3:271:7 | call to scanf | call to scanf |
12-
| test.cpp:280:7:280:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:279:3:279:7 | call to scanf | call to scanf |
13-
| test.cpp:292:7:292:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:291:3:291:7 | call to scanf | call to scanf |
14-
| test.cpp:404:25:404:25 | u | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:403:6:403:11 | call to sscanf | call to sscanf |
15-
| test.cpp:416:7:416:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:413:7:413:11 | call to scanf | call to scanf |
16-
| test.cpp:423:7:423:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:420:7:420:11 | call to scanf | call to scanf |
17-
| test.cpp:460:6:460:10 | value | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:455:12:455:17 | call to sscanf | call to sscanf |
18-
| test.cpp:474:6:474:10 | value | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:467:8:467:12 | call to scanf | call to scanf |
1+
edges
2+
| test.cpp:34:15:34:16 | scanf output argument | test.cpp:35:7:35:7 | i | provenance | |
3+
| test.cpp:41:19:41:20 | scanf output argument | test.cpp:43:8:43:8 | i | provenance | |
4+
| test.cpp:58:19:58:20 | scanf output argument | test.cpp:60:8:60:8 | i | provenance | |
5+
| test.cpp:67:15:67:16 | scanf output argument | test.cpp:68:7:68:7 | i | provenance | |
6+
| test.cpp:70:19:70:20 | scanf output argument | test.cpp:72:8:72:8 | i | provenance | |
7+
| test.cpp:79:15:79:16 | scanf output argument | test.cpp:80:7:80:7 | i | provenance | |
8+
| test.cpp:89:15:89:15 | scanf output argument | test.cpp:90:7:90:8 | * ... | provenance | |
9+
| test.cpp:97:15:97:15 | scanf output argument | test.cpp:98:7:98:8 | * ... | provenance | |
10+
| test.cpp:107:32:107:33 | fscanf output argument | test.cpp:108:7:108:7 | i | provenance | |
11+
| test.cpp:114:32:114:33 | sscanf output argument | test.cpp:115:7:115:7 | i | provenance | |
12+
| test.cpp:121:38:121:39 | _scanf_l output argument | test.cpp:123:8:123:8 | i | provenance | |
13+
| test.cpp:132:19:132:20 | scanf output argument | test.cpp:134:8:134:8 | i | provenance | |
14+
| test.cpp:141:19:141:20 | scanf output argument | test.cpp:143:8:143:8 | i | provenance | |
15+
| test.cpp:150:23:150:24 | scanf output argument | test.cpp:154:9:154:9 | i | provenance | |
16+
| test.cpp:181:19:181:20 | scanf output argument | test.cpp:185:8:185:8 | i | provenance | |
17+
| test.cpp:193:19:193:20 | scanf output argument | test.cpp:197:8:197:8 | i | provenance | |
18+
| test.cpp:211:22:211:23 | scanf output argument | test.cpp:213:8:213:8 | i | provenance | |
19+
| test.cpp:221:22:221:23 | scanf output argument | test.cpp:223:8:223:8 | i | provenance | |
20+
| test.cpp:221:26:221:27 | scanf output argument | test.cpp:224:8:224:8 | j | provenance | |
21+
| test.cpp:231:22:231:23 | scanf output argument | test.cpp:233:8:233:8 | i | provenance | |
22+
| test.cpp:231:26:231:27 | scanf output argument | test.cpp:234:8:234:8 | j | provenance | |
23+
| test.cpp:246:44:246:45 | scanf output argument | test.cpp:248:9:248:9 | d | provenance | |
24+
| test.cpp:250:33:250:34 | scanf output argument | test.cpp:252:9:252:9 | d | provenance | |
25+
| test.cpp:271:15:271:16 | scanf output argument | test.cpp:272:7:272:7 | i | provenance | |
26+
| test.cpp:279:15:279:16 | scanf output argument | test.cpp:280:7:280:7 | i | provenance | |
27+
| test.cpp:291:15:291:16 | scanf output argument | test.cpp:292:7:292:7 | i | provenance | |
28+
| test.cpp:325:34:325:35 | sscanf output argument | test.cpp:327:8:327:8 | i | provenance | |
29+
| test.cpp:325:38:325:39 | sscanf output argument | test.cpp:328:8:328:8 | j | provenance | |
30+
| test.cpp:335:22:335:23 | scanf output argument | test.cpp:337:8:337:8 | i | provenance | |
31+
| test.cpp:344:23:344:24 | scanf output argument | test.cpp:346:8:346:8 | i | provenance | |
32+
| test.cpp:353:26:353:27 | scanf output argument | test.cpp:354:8:354:8 | d | provenance | |
33+
| test.cpp:353:30:353:31 | scanf output argument | test.cpp:355:8:355:8 | n | provenance | |
34+
| test.cpp:362:62:362:63 | sscanf output argument | test.cpp:364:17:364:17 | n | provenance | |
35+
| test.cpp:403:29:403:30 | sscanf output argument | test.cpp:404:18:404:25 | u | provenance | |
36+
| test.cpp:413:19:413:20 | scanf output argument | test.cpp:416:7:416:7 | i | provenance | |
37+
| test.cpp:420:19:420:20 | scanf output argument | test.cpp:423:7:423:7 | i | provenance | |
38+
| test.cpp:455:41:455:46 | sscanf output argument | test.cpp:460:6:460:10 | value | provenance | |
39+
| test.cpp:467:20:467:25 | scanf output argument | test.cpp:474:6:474:10 | value | provenance | |
40+
nodes
41+
| test.cpp:34:15:34:16 | scanf output argument | semmle.label | scanf output argument |
42+
| test.cpp:35:7:35:7 | i | semmle.label | i |
43+
| test.cpp:41:19:41:20 | scanf output argument | semmle.label | scanf output argument |
44+
| test.cpp:43:8:43:8 | i | semmle.label | i |
45+
| test.cpp:58:19:58:20 | scanf output argument | semmle.label | scanf output argument |
46+
| test.cpp:60:8:60:8 | i | semmle.label | i |
47+
| test.cpp:67:15:67:16 | scanf output argument | semmle.label | scanf output argument |
48+
| test.cpp:68:7:68:7 | i | semmle.label | i |
49+
| test.cpp:70:19:70:20 | scanf output argument | semmle.label | scanf output argument |
50+
| test.cpp:72:8:72:8 | i | semmle.label | i |
51+
| test.cpp:79:15:79:16 | scanf output argument | semmle.label | scanf output argument |
52+
| test.cpp:80:7:80:7 | i | semmle.label | i |
53+
| test.cpp:89:15:89:15 | scanf output argument | semmle.label | scanf output argument |
54+
| test.cpp:90:7:90:8 | * ... | semmle.label | * ... |
55+
| test.cpp:97:15:97:15 | scanf output argument | semmle.label | scanf output argument |
56+
| test.cpp:98:7:98:8 | * ... | semmle.label | * ... |
57+
| test.cpp:107:32:107:33 | fscanf output argument | semmle.label | fscanf output argument |
58+
| test.cpp:108:7:108:7 | i | semmle.label | i |
59+
| test.cpp:114:32:114:33 | sscanf output argument | semmle.label | sscanf output argument |
60+
| test.cpp:115:7:115:7 | i | semmle.label | i |
61+
| test.cpp:121:38:121:39 | _scanf_l output argument | semmle.label | _scanf_l output argument |
62+
| test.cpp:123:8:123:8 | i | semmle.label | i |
63+
| test.cpp:132:19:132:20 | scanf output argument | semmle.label | scanf output argument |
64+
| test.cpp:134:8:134:8 | i | semmle.label | i |
65+
| test.cpp:141:19:141:20 | scanf output argument | semmle.label | scanf output argument |
66+
| test.cpp:143:8:143:8 | i | semmle.label | i |
67+
| test.cpp:150:23:150:24 | scanf output argument | semmle.label | scanf output argument |
68+
| test.cpp:154:9:154:9 | i | semmle.label | i |
69+
| test.cpp:181:19:181:20 | scanf output argument | semmle.label | scanf output argument |
70+
| test.cpp:185:8:185:8 | i | semmle.label | i |
71+
| test.cpp:193:19:193:20 | scanf output argument | semmle.label | scanf output argument |
72+
| test.cpp:197:8:197:8 | i | semmle.label | i |
73+
| test.cpp:211:22:211:23 | scanf output argument | semmle.label | scanf output argument |
74+
| test.cpp:213:8:213:8 | i | semmle.label | i |
75+
| test.cpp:221:22:221:23 | scanf output argument | semmle.label | scanf output argument |
76+
| test.cpp:221:26:221:27 | scanf output argument | semmle.label | scanf output argument |
77+
| test.cpp:223:8:223:8 | i | semmle.label | i |
78+
| test.cpp:224:8:224:8 | j | semmle.label | j |
79+
| test.cpp:231:22:231:23 | scanf output argument | semmle.label | scanf output argument |
80+
| test.cpp:231:26:231:27 | scanf output argument | semmle.label | scanf output argument |
81+
| test.cpp:233:8:233:8 | i | semmle.label | i |
82+
| test.cpp:234:8:234:8 | j | semmle.label | j |
83+
| test.cpp:246:44:246:45 | scanf output argument | semmle.label | scanf output argument |
84+
| test.cpp:248:9:248:9 | d | semmle.label | d |
85+
| test.cpp:250:33:250:34 | scanf output argument | semmle.label | scanf output argument |
86+
| test.cpp:252:9:252:9 | d | semmle.label | d |
87+
| test.cpp:271:15:271:16 | scanf output argument | semmle.label | scanf output argument |
88+
| test.cpp:272:7:272:7 | i | semmle.label | i |
89+
| test.cpp:279:15:279:16 | scanf output argument | semmle.label | scanf output argument |
90+
| test.cpp:280:7:280:7 | i | semmle.label | i |
91+
| test.cpp:291:15:291:16 | scanf output argument | semmle.label | scanf output argument |
92+
| test.cpp:292:7:292:7 | i | semmle.label | i |
93+
| test.cpp:325:34:325:35 | sscanf output argument | semmle.label | sscanf output argument |
94+
| test.cpp:325:38:325:39 | sscanf output argument | semmle.label | sscanf output argument |
95+
| test.cpp:327:8:327:8 | i | semmle.label | i |
96+
| test.cpp:328:8:328:8 | j | semmle.label | j |
97+
| test.cpp:335:22:335:23 | scanf output argument | semmle.label | scanf output argument |
98+
| test.cpp:337:8:337:8 | i | semmle.label | i |
99+
| test.cpp:344:23:344:24 | scanf output argument | semmle.label | scanf output argument |
100+
| test.cpp:346:8:346:8 | i | semmle.label | i |
101+
| test.cpp:353:26:353:27 | scanf output argument | semmle.label | scanf output argument |
102+
| test.cpp:353:30:353:31 | scanf output argument | semmle.label | scanf output argument |
103+
| test.cpp:354:8:354:8 | d | semmle.label | d |
104+
| test.cpp:355:8:355:8 | n | semmle.label | n |
105+
| test.cpp:362:62:362:63 | sscanf output argument | semmle.label | sscanf output argument |
106+
| test.cpp:364:17:364:17 | n | semmle.label | n |
107+
| test.cpp:403:29:403:30 | sscanf output argument | semmle.label | sscanf output argument |
108+
| test.cpp:404:18:404:25 | u | semmle.label | u |
109+
| test.cpp:413:19:413:20 | scanf output argument | semmle.label | scanf output argument |
110+
| test.cpp:416:7:416:7 | i | semmle.label | i |
111+
| test.cpp:420:19:420:20 | scanf output argument | semmle.label | scanf output argument |
112+
| test.cpp:423:7:423:7 | i | semmle.label | i |
113+
| test.cpp:455:41:455:46 | sscanf output argument | semmle.label | sscanf output argument |
114+
| test.cpp:460:6:460:10 | value | semmle.label | value |
115+
| test.cpp:467:20:467:25 | scanf output argument | semmle.label | scanf output argument |
116+
| test.cpp:474:6:474:10 | value | semmle.label | value |
117+
subpaths
118+
#select
119+
| test.cpp:35:7:35:7 | i | test.cpp:34:15:34:16 | scanf output argument | test.cpp:35:7:35:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:34:3:34:7 | call to scanf | call to scanf |
120+
| test.cpp:68:7:68:7 | i | test.cpp:67:15:67:16 | scanf output argument | test.cpp:68:7:68:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:67:3:67:7 | call to scanf | call to scanf |
121+
| test.cpp:80:7:80:7 | i | test.cpp:79:15:79:16 | scanf output argument | test.cpp:80:7:80:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:79:3:79:7 | call to scanf | call to scanf |
122+
| test.cpp:90:7:90:8 | * ... | test.cpp:89:15:89:15 | scanf output argument | test.cpp:90:7:90:8 | * ... | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:89:3:89:7 | call to scanf | call to scanf |
123+
| test.cpp:98:7:98:8 | * ... | test.cpp:97:15:97:15 | scanf output argument | test.cpp:98:7:98:8 | * ... | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:97:3:97:7 | call to scanf | call to scanf |
124+
| test.cpp:108:7:108:7 | i | test.cpp:107:32:107:33 | fscanf output argument | test.cpp:108:7:108:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:107:3:107:8 | call to fscanf | call to fscanf |
125+
| test.cpp:115:7:115:7 | i | test.cpp:114:32:114:33 | sscanf output argument | test.cpp:115:7:115:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:114:3:114:8 | call to sscanf | call to sscanf |
126+
| test.cpp:224:8:224:8 | j | test.cpp:221:26:221:27 | scanf output argument | test.cpp:224:8:224:8 | j | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 2. | test.cpp:221:7:221:11 | call to scanf | call to scanf |
127+
| test.cpp:248:9:248:9 | d | test.cpp:246:44:246:45 | scanf output argument | test.cpp:248:9:248:9 | d | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 2. | test.cpp:246:25:246:29 | call to scanf | call to scanf |
128+
| test.cpp:252:9:252:9 | d | test.cpp:250:33:250:34 | scanf output argument | test.cpp:252:9:252:9 | d | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 2. | test.cpp:250:14:250:18 | call to scanf | call to scanf |
129+
| test.cpp:272:7:272:7 | i | test.cpp:271:15:271:16 | scanf output argument | test.cpp:272:7:272:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:271:3:271:7 | call to scanf | call to scanf |
130+
| test.cpp:280:7:280:7 | i | test.cpp:279:15:279:16 | scanf output argument | test.cpp:280:7:280:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:279:3:279:7 | call to scanf | call to scanf |
131+
| test.cpp:292:7:292:7 | i | test.cpp:291:15:291:16 | scanf output argument | test.cpp:292:7:292:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:291:3:291:7 | call to scanf | call to scanf |
132+
| test.cpp:404:25:404:25 | u | test.cpp:403:29:403:30 | sscanf output argument | test.cpp:404:18:404:25 | u | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:403:6:403:11 | call to sscanf | call to sscanf |
133+
| test.cpp:416:7:416:7 | i | test.cpp:413:19:413:20 | scanf output argument | test.cpp:416:7:416:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:413:7:413:11 | call to scanf | call to scanf |
134+
| test.cpp:423:7:423:7 | i | test.cpp:420:19:420:20 | scanf output argument | test.cpp:423:7:423:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:420:7:420:11 | call to scanf | call to scanf |
135+
| test.cpp:460:6:460:10 | value | test.cpp:455:41:455:46 | sscanf output argument | test.cpp:460:6:460:10 | value | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:455:12:455:17 | call to sscanf | call to sscanf |
136+
| test.cpp:474:6:474:10 | value | test.cpp:467:20:467:25 | scanf output argument | test.cpp:474:6:474:10 | value | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:467:8:467:12 | call to scanf | call to scanf |

0 commit comments

Comments
 (0)