Skip to content

Commit 6026f65

Browse files
geoffw0geoffw0
committed
C++: Fix FPs in cpp/incorrectly-checked-scanf.
1 parent c3f2faf commit 6026f65

File tree

4 files changed

+50
-12
lines changed

4 files changed

+50
-12
lines changed

‎cpp/ql/src/Critical/ScanfChecks.qll

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,18 @@ private string getEofValue() {
3838
private predicate checkedForEof(ScanfFunctionCall call) {
3939
exists(IRGuardCondition gc |
4040
exists(Instruction i | i.getUnconvertedResultExpression() = call |
41-
// call == EOF
42-
gc.comparesEq(valueNumber(i).getAUse(), getEofValue().toInt(), _, _)
41+
exists(int val | gc.comparesEq(valueNumber(i).getAUse(), val, _, _) |
42+
// call == EOF
43+
val = getEofValue().toInt()
44+
or
45+
// call == [any positive number]
46+
val > 0
47+
)
4348
or
44-
// call < 0 (EOF is guaranteed to be negative)
45-
gc.comparesLt(valueNumber(i).getAUse(), 0, true, _)
49+
exists(int val | gc.comparesLt(valueNumber(i).getAUse(), val, true, _) |
50+
// call < [any non-negative number] (EOF is guaranteed to be negative)
51+
val >= 0
52+
)
4653
)
4754
)
4855
}

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,3 @@
33
| test.cpp:204:7:204:11 | call to scanf | The result of scanf is only checked against 0, but it can also return EOF. |
44
| test.cpp:436:7:436:11 | call to scanf | The result of scanf is only checked against 0, but it can also return EOF. |
55
| test.cpp:443:11:443:15 | call to scanf | The result of scanf is only checked against 0, but it can also return EOF. |
6-
| test.cpp:501:13:501:17 | call to scanf | The result of scanf is only checked against 0, but it can also return EOF. |
7-
| test.cpp:512:13:512:17 | call to scanf | The result of scanf is only checked against 0, but it can also return EOF. |
8-
| test.cpp:525:10:525:15 | call to sscanf | The result of scanf is only checked against 0, but it can also return EOF. |
9-
| test.cpp:541:10:541:15 | call to sscanf | The result of scanf is only checked against 0, but it can also return EOF. |

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,19 @@ edges
3939
| test.cpp:467:20:467:25 | scanf output argument | test.cpp:474:6:474:10 | value | provenance | |
4040
| test.cpp:480:25:480:26 | scanf output argument | test.cpp:484:9:484:9 | i | provenance | |
4141
| test.cpp:491:25:491:26 | scanf output argument | test.cpp:495:8:495:8 | i | provenance | |
42+
| test.cpp:501:25:501:26 | scanf output argument | test.cpp:505:9:505:9 | i | provenance | |
43+
| test.cpp:512:25:512:26 | scanf output argument | test.cpp:516:9:516:9 | i | provenance | |
44+
| test.cpp:525:35:525:36 | sscanf output argument | test.cpp:527:8:527:8 | a | provenance | |
45+
| test.cpp:525:35:525:36 | sscanf output argument | test.cpp:531:8:531:8 | a | provenance | |
46+
| test.cpp:525:39:525:40 | sscanf output argument | test.cpp:528:8:528:8 | b | provenance | |
47+
| test.cpp:525:39:525:40 | sscanf output argument | test.cpp:532:8:532:8 | b | provenance | |
48+
| test.cpp:525:43:525:44 | sscanf output argument | test.cpp:533:8:533:8 | c | provenance | |
49+
| test.cpp:541:35:541:36 | sscanf output argument | test.cpp:543:8:543:8 | d | provenance | |
50+
| test.cpp:541:35:541:36 | sscanf output argument | test.cpp:548:8:548:8 | d | provenance | |
51+
| test.cpp:541:39:541:40 | sscanf output argument | test.cpp:544:8:544:8 | e | provenance | |
52+
| test.cpp:541:39:541:40 | sscanf output argument | test.cpp:549:8:549:8 | e | provenance | |
53+
| test.cpp:541:43:541:44 | sscanf output argument | test.cpp:545:8:545:8 | f | provenance | |
54+
| test.cpp:541:43:541:44 | sscanf output argument | test.cpp:550:8:550:8 | f | provenance | |
4255
nodes
4356
| test.cpp:34:15:34:16 | scanf output argument | semmle.label | scanf output argument |
4457
| test.cpp:35:7:35:7 | i | semmle.label | i |
@@ -120,6 +133,27 @@ nodes
120133
| test.cpp:484:9:484:9 | i | semmle.label | i |
121134
| test.cpp:491:25:491:26 | scanf output argument | semmle.label | scanf output argument |
122135
| test.cpp:495:8:495:8 | i | semmle.label | i |
136+
| test.cpp:501:25:501:26 | scanf output argument | semmle.label | scanf output argument |
137+
| test.cpp:505:9:505:9 | i | semmle.label | i |
138+
| test.cpp:512:25:512:26 | scanf output argument | semmle.label | scanf output argument |
139+
| test.cpp:516:9:516:9 | i | semmle.label | i |
140+
| test.cpp:525:35:525:36 | sscanf output argument | semmle.label | sscanf output argument |
141+
| test.cpp:525:39:525:40 | sscanf output argument | semmle.label | sscanf output argument |
142+
| test.cpp:525:43:525:44 | sscanf output argument | semmle.label | sscanf output argument |
143+
| test.cpp:527:8:527:8 | a | semmle.label | a |
144+
| test.cpp:528:8:528:8 | b | semmle.label | b |
145+
| test.cpp:531:8:531:8 | a | semmle.label | a |
146+
| test.cpp:532:8:532:8 | b | semmle.label | b |
147+
| test.cpp:533:8:533:8 | c | semmle.label | c |
148+
| test.cpp:541:35:541:36 | sscanf output argument | semmle.label | sscanf output argument |
149+
| test.cpp:541:39:541:40 | sscanf output argument | semmle.label | sscanf output argument |
150+
| test.cpp:541:43:541:44 | sscanf output argument | semmle.label | sscanf output argument |
151+
| test.cpp:543:8:543:8 | d | semmle.label | d |
152+
| test.cpp:544:8:544:8 | e | semmle.label | e |
153+
| test.cpp:545:8:545:8 | f | semmle.label | f |
154+
| test.cpp:548:8:548:8 | d | semmle.label | d |
155+
| test.cpp:549:8:549:8 | e | semmle.label | e |
156+
| test.cpp:550:8:550:8 | f | semmle.label | f |
123157
subpaths
124158
#select
125159
| 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 |
@@ -142,3 +176,4 @@ subpaths
142176
| 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 |
143177
| test.cpp:484:9:484:9 | i | test.cpp:480:25:480:26 | scanf output argument | test.cpp:484:9:484:9 | 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:480:13:480:17 | call to scanf | call to scanf |
144178
| test.cpp:495:8:495:8 | i | test.cpp:491:25:491:26 | scanf output argument | test.cpp:495:8:495:8 | 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:491:13:491:17 | call to scanf | call to scanf |
179+
| test.cpp:545:8:545:8 | f | test.cpp:541:43:541:44 | sscanf output argument | test.cpp:545:8:545:8 | f | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 3. | test.cpp:541:10:541:15 | call to sscanf | call to sscanf |

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -498,7 +498,7 @@ void multiple_checks() {
498498

499499
{
500500
int i;
501-
int res = scanf("%d", &i); // [FALSE POSITIVE]
501+
int res = scanf("%d", &i);
502502

503503
if (res >= 1) {
504504
if (res != 0) {
@@ -509,7 +509,7 @@ void multiple_checks() {
509509

510510
{
511511
int i;
512-
int res = scanf("%d", &i); // [FALSE POSITIVE]
512+
int res = scanf("%d", &i);
513513

514514
if (res == 1) {
515515
if (res != 0) {
@@ -522,7 +522,7 @@ void multiple_checks() {
522522
void switch_cases(const char *data) {
523523
float a, b, c;
524524

525-
switch (sscanf(data, "%f %f %f", &a, &b, &c)) { // [FALSE POSITIVE]
525+
switch (sscanf(data, "%f %f %f", &a, &b, &c)) {
526526
case 2:
527527
use(a); // GOOD
528528
use(b); // GOOD
@@ -538,7 +538,7 @@ void switch_cases(const char *data) {
538538

539539
float d, e, f;
540540

541-
switch (sscanf(data, "%f %f %f", &d, &e, &f)) { // [REPORTED HERE]
541+
switch (sscanf(data, "%f %f %f", &d, &e, &f)) {
542542
case 2:
543543
use(d); // GOOD
544544
use(e); // GOOD

0 commit comments

Comments
 (0)