From d8807116b391946a814e20395d737718ed7c7a57 Mon Sep 17 00:00:00 2001 From: SkyCipherr Date: Sun, 11 Oct 2026 19:28:15 +0800 Subject: [PATCH] C++: reduce scanf false positives after fallback initialization --- cpp/ql/src/Critical/MissingCheckScanf.ql | 45 +++++ ...026-10-10-missing-check-scanf-overwrite.md | 4 + .../MissingCheckScanf.expected | 187 ++++++++---------- .../Critical/MissingCheckScanf/overwrites.c | 181 +++++++++++++++++ .../Critical/MissingCheckScanf/overwrites.cpp | 49 +++++ .../Critical/MissingCheckScanf/test.cpp | 8 +- 6 files changed, 367 insertions(+), 107 deletions(-) create mode 100644 cpp/ql/src/change-notes/2026-10-10-missing-check-scanf-overwrite.md create mode 100644 cpp/ql/test/query-tests/Critical/MissingCheckScanf/overwrites.c create mode 100644 cpp/ql/test/query-tests/Critical/MissingCheckScanf/overwrites.cpp diff --git a/cpp/ql/src/Critical/MissingCheckScanf.ql b/cpp/ql/src/Critical/MissingCheckScanf.ql index 5d8ce9e88469..2ebe1ac9586f 100644 --- a/cpp/ql/src/Critical/MissingCheckScanf.ql +++ b/cpp/ql/src/Critical/MissingCheckScanf.ql @@ -16,8 +16,10 @@ import cpp import semmle.code.cpp.commons.Scanf import semmle.code.cpp.controlflow.Guards +import semmle.code.cpp.controlflow.IRGuards as IRGuards import semmle.code.cpp.dataflow.new.DataFlow::DataFlow import semmle.code.cpp.ir.IR +import semmle.code.cpp.ir.ValueNumbering import semmle.code.cpp.valuenumbering.GlobalValueNumbering import ScanfChecks import ScanfToUseFlow::PathGraph @@ -103,6 +105,10 @@ module ScanfToUseConfig implements ConfigSig { predicate isSink(Node sink) { isSink(sink, _) } + predicate isBarrier(Node node) { + node = BarrierGuard::getAnIndirectBarrierNode(1) + } + predicate isBarrierOut(Node n) { // We disable flow out of sinks to reduce result duplication isSink(n, _) @@ -143,6 +149,45 @@ int getMinimumGuardConstant(ScanfFunctionCall call, int index) { ) } +/** + * Holds if `output` addresses a local non-reference variable whose other lvalue uses are + * assignments, increments, or output arguments of `scanf`-like calls. + * Aliased reads are not always tracked by this configuration, so retaining + * the successful flow for other address uses avoids hiding an unsafe read. + */ +private predicate isUnaliasedScanfOutput(Expr output) { + exists(LocalVariable variable, AddressOfExpr address | + output.getUnconverted() = address and + address.getOperand().(VariableAccess).getTarget() = variable and + not variable.getType().getUnspecifiedType() instanceof ReferenceType and + forall(VariableAccess access | + access.getTarget() = variable and access.isUsedAsLValue() and not access.isModified() + | + exists(ScanfFunctionCall call, int index | + call.getOutputArgument(index).getUnconverted() = access.getParent() + ) + ) + ) +} + +/** Holds if `guard` ensures that `output` was written when it evaluates to `branch`. */ +private predicate scanfChecksOutput(IRGuards::IRGuardCondition guard, Expr output, boolean branch) { + exists( + ScanfFunctionCall call, int index, CallInstruction instruction, int minGuard, + IRGuards::GuardValue value + | + value.asBooleanValue() = branch and + output = call.getOutputArgument(index).getFullyConverted() and + isUnaliasedScanfOutput(output) and + instruction.getUnconvertedResultExpression() = call and + minGuard = getMinimumGuardConstant(call, index) + | + guard.comparesEq(valueNumber(instruction).getAUse(), any(int k | minGuard <= k), true, value) + or + guard.comparesLt(valueNumber(instruction).getAUse(), any(int k | minGuard <= k), false, value) + ) +} + /** * Holds the access to `e` isn't guarded by a check that ensures that `call` returned * at least `minGuard`. diff --git a/cpp/ql/src/change-notes/2026-10-10-missing-check-scanf-overwrite.md b/cpp/ql/src/change-notes/2026-10-10-missing-check-scanf-overwrite.md new file mode 100644 index 000000000000..6fa9c17b0cd9 --- /dev/null +++ b/cpp/ql/src/change-notes/2026-10-10-missing-check-scanf-overwrite.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The `cpp/missing-check-scanf` query produces fewer false positives when an unaliased local output variable is overwritten with a default value if a `scanf`-like call fails. diff --git a/cpp/ql/test/query-tests/Critical/MissingCheckScanf/MissingCheckScanf.expected b/cpp/ql/test/query-tests/Critical/MissingCheckScanf/MissingCheckScanf.expected index 9b7564b9123d..81c388549255 100644 --- a/cpp/ql/test/query-tests/Critical/MissingCheckScanf/MissingCheckScanf.expected +++ b/cpp/ql/test/query-tests/Critical/MissingCheckScanf/MissingCheckScanf.expected @@ -1,46 +1,74 @@ +#select +| overwrites.c:65:9:65:9 | j | overwrites.c:60:28:60:29 | scanf output argument | overwrites.c:65:9:65:9 | j | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 2. | overwrites.c:60:9:60:13 | call to scanf | call to scanf | +| overwrites.c:93:9:93:9 | i | overwrites.c:88:21:88:22 | scanf output argument | overwrites.c:93:9:93: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. | overwrites.c:88:9:88:13 | call to scanf | call to scanf | +| overwrites.c:101:9:101:9 | i | overwrites.c:98:21:98:22 | scanf output argument | overwrites.c:101:9:101: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. | overwrites.c:98:9:98:13 | call to scanf | call to scanf | +| overwrites.c:107:13:107:13 | i | overwrites.c:106:21:106:22 | scanf output argument | overwrites.c:107:13:107:13 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | overwrites.c:106:9:106:13 | call to scanf | call to scanf | +| overwrites.c:115:11:115:11 | i | overwrites.c:114:21:114:22 | scanf output argument | overwrites.c:115:11:115:11 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | overwrites.c:114:9:114:13 | call to scanf | call to scanf | +| overwrites.c:126:9:126:9 | i | overwrites.c:123:21:123:22 | scanf output argument | overwrites.c:126:9:126: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. | overwrites.c:123:9:123:13 | call to scanf | call to scanf | +| overwrites.c:132:13:132:13 | i | overwrites.c:131:21:131:22 | scanf output argument | overwrites.c:132:13:132:13 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | overwrites.c:131:9:131:13 | call to scanf | call to scanf | +| overwrites.c:142:13:142:13 | i | overwrites.c:140:17:140:18 | scanf output argument | overwrites.c:142:13:142:13 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | overwrites.c:140:5:140:9 | call to scanf | call to scanf | +| overwrites.c:149:9:149:9 | i | overwrites.c:148:17:148:18 | scanf output argument | overwrites.c:149:9:149: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. | overwrites.c:148:5:148:9 | call to scanf | call to scanf | +| overwrites.c:163:9:163:9 | i | overwrites.c:158:30:158:31 | scanf output argument | overwrites.c:163:9:163: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. | overwrites.c:158:18:158:22 | call to scanf | call to scanf | +| overwrites.c:172:9:172:9 | j | overwrites.c:168:28:168:29 | scanf output argument | overwrites.c:172:9:172:9 | j | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 2. | overwrites.c:168:9:168:13 | call to scanf | call to scanf | +| overwrites.c:178:23:178:23 | i | overwrites.c:177:21:177:22 | scanf output argument | overwrites.c:178:23:178:23 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | overwrites.c:177:9:177:13 | call to scanf | call to scanf | +| overwrites.cpp:10:9:10:13 | value | overwrites.cpp:7:21:7:26 | scanf output argument | overwrites.cpp:10:9:10:13 | value | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | overwrites.cpp:7:9:7:13 | call to scanf | call to scanf | +| overwrites.cpp:19:9:19:13 | value | overwrites.cpp:16:21:16:26 | scanf output argument | overwrites.cpp:19:9:19:13 | value | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | overwrites.cpp:16:9:16:13 | call to scanf | call to scanf | +| overwrites.cpp:38:9:38:14 | output | overwrites.cpp:35:21:35:27 | scanf output argument | overwrites.cpp:38:9:38:14 | output | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | overwrites.cpp:35:9:35:13 | call to scanf | call to scanf | +| overwrites.cpp:48:9:48:14 | output | overwrites.cpp:45:21:45:27 | scanf output argument | overwrites.cpp:48:9:48:14 | output | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | overwrites.cpp:45:9:45:13 | call to scanf | call to scanf | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | +| 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 | edges -| test.c:10:31:10:32 | sscanf output argument | test.c:11:7:11:7 | x | provenance | | +| overwrites.c:60:28:60:29 | scanf output argument | overwrites.c:65:9:65:9 | j | provenance | | +| overwrites.c:88:21:88:22 | scanf output argument | overwrites.c:93:9:93:9 | i | provenance | | +| overwrites.c:98:21:98:22 | scanf output argument | overwrites.c:101:9:101:9 | i | provenance | | +| overwrites.c:106:21:106:22 | scanf output argument | overwrites.c:107:13:107:13 | i | provenance | | +| overwrites.c:114:21:114:22 | scanf output argument | overwrites.c:115:11:115:11 | i | provenance | | +| overwrites.c:123:21:123:22 | scanf output argument | overwrites.c:126:9:126:9 | i | provenance | | +| overwrites.c:131:21:131:22 | scanf output argument | overwrites.c:132:13:132:13 | i | provenance | | +| overwrites.c:140:17:140:18 | scanf output argument | overwrites.c:142:13:142:13 | i | provenance | | +| overwrites.c:148:17:148:18 | scanf output argument | overwrites.c:149:9:149:9 | i | provenance | | +| overwrites.c:158:30:158:31 | scanf output argument | overwrites.c:163:9:163:9 | i | provenance | | +| overwrites.c:168:28:168:29 | scanf output argument | overwrites.c:172:9:172:9 | j | provenance | | +| overwrites.c:177:21:177:22 | scanf output argument | overwrites.c:178:23:178:23 | i | provenance | | +| overwrites.cpp:7:21:7:26 | scanf output argument | overwrites.cpp:10:9:10:13 | value | provenance | | +| overwrites.cpp:16:21:16:26 | scanf output argument | overwrites.cpp:19:9:19:13 | value | provenance | | +| overwrites.cpp:35:21:35:27 | scanf output argument | overwrites.cpp:38:9:38:14 | output | provenance | | +| overwrites.cpp:45:21:45:27 | scanf output argument | overwrites.cpp:48:9:48:14 | output | provenance | | | test.cpp:34:15:34:16 | scanf output argument | test.cpp:35:7:35:7 | i | provenance | | -| test.cpp:41:19:41:20 | scanf output argument | test.cpp:43:8:43:8 | i | provenance | | -| test.cpp:58:19:58:20 | scanf output argument | test.cpp:60:8:60:8 | i | provenance | | | test.cpp:67:15:67:16 | scanf output argument | test.cpp:68:7:68:7 | i | provenance | | -| test.cpp:70:19:70:20 | scanf output argument | test.cpp:72:8:72:8 | i | provenance | | | test.cpp:79:15:79:16 | scanf output argument | test.cpp:80:7:80:7 | i | provenance | | | test.cpp:89:15:89:15 | scanf output argument | test.cpp:90:7:90:8 | * ... | provenance | | | test.cpp:97:15:97:15 | scanf output argument | test.cpp:98:7:98:8 | * ... | provenance | | | test.cpp:107:32:107:33 | fscanf output argument | test.cpp:108:7:108:7 | i | provenance | | | test.cpp:114:32:114:33 | sscanf output argument | test.cpp:115:7:115:7 | i | provenance | | -| test.cpp:121:38:121:39 | _scanf_l output argument | test.cpp:123:8:123:8 | i | provenance | | -| test.cpp:132:19:132:20 | scanf output argument | test.cpp:134:8:134:8 | i | provenance | | -| test.cpp:141:19:141:20 | scanf output argument | test.cpp:143:8:143:8 | i | provenance | | -| test.cpp:150:23:150:24 | scanf output argument | test.cpp:154:9:154:9 | i | provenance | | -| test.cpp:181:19:181:20 | scanf output argument | test.cpp:185:8:185:8 | i | provenance | | -| test.cpp:211:22:211:23 | scanf output argument | test.cpp:213:8:213:8 | i | provenance | | -| test.cpp:221:22:221:23 | scanf output argument | test.cpp:223:8:223:8 | i | provenance | | | test.cpp:221:26:221:27 | scanf output argument | test.cpp:224:8:224:8 | j | provenance | | -| test.cpp:231:22:231:23 | scanf output argument | test.cpp:233:8:233:8 | i | provenance | | -| test.cpp:231:26:231:27 | scanf output argument | test.cpp:234:8:234:8 | j | provenance | | | test.cpp:246:44:246:45 | scanf output argument | test.cpp:248:9:248:9 | d | provenance | | | test.cpp:250:33:250:34 | scanf output argument | test.cpp:252:9:252:9 | d | provenance | | | test.cpp:271:15:271:16 | scanf output argument | test.cpp:272:7:272:7 | i | provenance | | | test.cpp:279:15:279:16 | scanf output argument | test.cpp:280:7:280:7 | i | provenance | | | test.cpp:291:15:291:16 | scanf output argument | test.cpp:292:7:292:7 | i | provenance | | -| test.cpp:325:34:325:35 | sscanf output argument | test.cpp:327:8:327:8 | i | provenance | | -| test.cpp:325:38:325:39 | sscanf output argument | test.cpp:328:8:328:8 | j | provenance | | -| test.cpp:335:22:335:23 | scanf output argument | test.cpp:337:8:337:8 | i | provenance | | -| test.cpp:344:23:344:24 | scanf output argument | test.cpp:346:8:346:8 | i | provenance | | -| test.cpp:353:26:353:27 | scanf output argument | test.cpp:354:8:354:8 | d | provenance | | -| test.cpp:353:30:353:31 | scanf output argument | test.cpp:355:8:355:8 | n | provenance | | -| test.cpp:362:62:362:63 | sscanf output argument | test.cpp:364:17:364:17 | n | provenance | | | test.cpp:403:29:403:30 | sscanf output argument | test.cpp:404:18:404:25 | u | provenance | | -| test.cpp:413:19:413:20 | scanf output argument | test.cpp:416:7:416:7 | i | provenance | | -| test.cpp:420:19:420:20 | scanf output argument | test.cpp:423:7:423:7 | i | provenance | | | test.cpp:455:41:455:46 | sscanf output argument | test.cpp:460:6:460:10 | value | provenance | | | test.cpp:467:20:467:25 | scanf output argument | test.cpp:474:6:474:10 | value | provenance | | | test.cpp:480:25:480:26 | scanf output argument | test.cpp:484:9:484:9 | i | provenance | | | test.cpp:491:25:491:26 | scanf output argument | test.cpp:495:8:495:8 | i | provenance | | -| test.cpp:501:25:501:26 | scanf output argument | test.cpp:505:9:505:9 | i | provenance | | -| test.cpp:512:25:512:26 | scanf output argument | test.cpp:516:9:516:9 | i | provenance | | | test.cpp:525:35:525:36 | sscanf output argument | test.cpp:527:8:527:8 | a | provenance | | | test.cpp:525:35:525:36 | sscanf output argument | test.cpp:531:8:531:8 | a | provenance | | | test.cpp:525:39:525:40 | sscanf output argument | test.cpp:528:8:528:8 | b | provenance | | @@ -52,22 +80,43 @@ edges | test.cpp:541:39:541:40 | sscanf output argument | test.cpp:549:8:549:8 | e | provenance | | | test.cpp:541:43:541:44 | sscanf output argument | test.cpp:545:8:545:8 | f | provenance | | | test.cpp:541:43:541:44 | sscanf output argument | test.cpp:550:8:550:8 | f | provenance | | -| test.cpp:559:30:559:31 | scanf output argument | test.cpp:561:9:561:9 | i | provenance | | -| test.cpp:567:35:567:36 | scanf output argument | test.cpp:569:9:569:9 | i | provenance | | -| test.cpp:575:30:575:31 | scanf output argument | test.cpp:577:9:577:9 | i | provenance | | nodes -| test.c:10:31:10:32 | sscanf output argument | semmle.label | sscanf output argument | -| test.c:11:7:11:7 | x | semmle.label | x | +| overwrites.c:60:28:60:29 | scanf output argument | semmle.label | scanf output argument | +| overwrites.c:65:9:65:9 | j | semmle.label | j | +| overwrites.c:88:21:88:22 | scanf output argument | semmle.label | scanf output argument | +| overwrites.c:93:9:93:9 | i | semmle.label | i | +| overwrites.c:98:21:98:22 | scanf output argument | semmle.label | scanf output argument | +| overwrites.c:101:9:101:9 | i | semmle.label | i | +| overwrites.c:106:21:106:22 | scanf output argument | semmle.label | scanf output argument | +| overwrites.c:107:13:107:13 | i | semmle.label | i | +| overwrites.c:114:21:114:22 | scanf output argument | semmle.label | scanf output argument | +| overwrites.c:115:11:115:11 | i | semmle.label | i | +| overwrites.c:123:21:123:22 | scanf output argument | semmle.label | scanf output argument | +| overwrites.c:126:9:126:9 | i | semmle.label | i | +| overwrites.c:131:21:131:22 | scanf output argument | semmle.label | scanf output argument | +| overwrites.c:132:13:132:13 | i | semmle.label | i | +| overwrites.c:140:17:140:18 | scanf output argument | semmle.label | scanf output argument | +| overwrites.c:142:13:142:13 | i | semmle.label | i | +| overwrites.c:148:17:148:18 | scanf output argument | semmle.label | scanf output argument | +| overwrites.c:149:9:149:9 | i | semmle.label | i | +| overwrites.c:158:30:158:31 | scanf output argument | semmle.label | scanf output argument | +| overwrites.c:163:9:163:9 | i | semmle.label | i | +| overwrites.c:168:28:168:29 | scanf output argument | semmle.label | scanf output argument | +| overwrites.c:172:9:172:9 | j | semmle.label | j | +| overwrites.c:177:21:177:22 | scanf output argument | semmle.label | scanf output argument | +| overwrites.c:178:23:178:23 | i | semmle.label | i | +| overwrites.cpp:7:21:7:26 | scanf output argument | semmle.label | scanf output argument | +| overwrites.cpp:10:9:10:13 | value | semmle.label | value | +| overwrites.cpp:16:21:16:26 | scanf output argument | semmle.label | scanf output argument | +| overwrites.cpp:19:9:19:13 | value | semmle.label | value | +| overwrites.cpp:35:21:35:27 | scanf output argument | semmle.label | scanf output argument | +| overwrites.cpp:38:9:38:14 | output | semmle.label | output | +| overwrites.cpp:45:21:45:27 | scanf output argument | semmle.label | scanf output argument | +| overwrites.cpp:48:9:48:14 | output | semmle.label | output | | test.cpp:34:15:34:16 | scanf output argument | semmle.label | scanf output argument | | test.cpp:35:7:35:7 | i | semmle.label | i | -| test.cpp:41:19:41:20 | scanf output argument | semmle.label | scanf output argument | -| test.cpp:43:8:43:8 | i | semmle.label | i | -| test.cpp:58:19:58:20 | scanf output argument | semmle.label | scanf output argument | -| test.cpp:60:8:60:8 | i | semmle.label | i | | test.cpp:67:15:67:16 | scanf output argument | semmle.label | scanf output argument | | test.cpp:68:7:68:7 | i | semmle.label | i | -| test.cpp:70:19:70:20 | scanf output argument | semmle.label | scanf output argument | -| test.cpp:72:8:72:8 | i | semmle.label | i | | test.cpp:79:15:79:16 | scanf output argument | semmle.label | scanf output argument | | test.cpp:80:7:80:7 | i | semmle.label | i | | test.cpp:89:15:89:15 | scanf output argument | semmle.label | scanf output argument | @@ -78,26 +127,8 @@ nodes | test.cpp:108:7:108:7 | i | semmle.label | i | | test.cpp:114:32:114:33 | sscanf output argument | semmle.label | sscanf output argument | | test.cpp:115:7:115:7 | i | semmle.label | i | -| test.cpp:121:38:121:39 | _scanf_l output argument | semmle.label | _scanf_l output argument | -| test.cpp:123:8:123:8 | i | semmle.label | i | -| test.cpp:132:19:132:20 | scanf output argument | semmle.label | scanf output argument | -| test.cpp:134:8:134:8 | i | semmle.label | i | -| test.cpp:141:19:141:20 | scanf output argument | semmle.label | scanf output argument | -| test.cpp:143:8:143:8 | i | semmle.label | i | -| test.cpp:150:23:150:24 | scanf output argument | semmle.label | scanf output argument | -| test.cpp:154:9:154:9 | i | semmle.label | i | -| test.cpp:181:19:181:20 | scanf output argument | semmle.label | scanf output argument | -| test.cpp:185:8:185:8 | i | semmle.label | i | -| test.cpp:211:22:211:23 | scanf output argument | semmle.label | scanf output argument | -| test.cpp:213:8:213:8 | i | semmle.label | i | -| test.cpp:221:22:221:23 | scanf output argument | semmle.label | scanf output argument | | test.cpp:221:26:221:27 | scanf output argument | semmle.label | scanf output argument | -| test.cpp:223:8:223:8 | i | semmle.label | i | | test.cpp:224:8:224:8 | j | semmle.label | j | -| test.cpp:231:22:231:23 | scanf output argument | semmle.label | scanf output argument | -| test.cpp:231:26:231:27 | scanf output argument | semmle.label | scanf output argument | -| test.cpp:233:8:233:8 | i | semmle.label | i | -| test.cpp:234:8:234:8 | j | semmle.label | j | | test.cpp:246:44:246:45 | scanf output argument | semmle.label | scanf output argument | | test.cpp:248:9:248:9 | d | semmle.label | d | | test.cpp:250:33:250:34 | scanf output argument | semmle.label | scanf output argument | @@ -108,26 +139,8 @@ nodes | test.cpp:280:7:280:7 | i | semmle.label | i | | test.cpp:291:15:291:16 | scanf output argument | semmle.label | scanf output argument | | test.cpp:292:7:292:7 | i | semmle.label | i | -| test.cpp:325:34:325:35 | sscanf output argument | semmle.label | sscanf output argument | -| test.cpp:325:38:325:39 | sscanf output argument | semmle.label | sscanf output argument | -| test.cpp:327:8:327:8 | i | semmle.label | i | -| test.cpp:328:8:328:8 | j | semmle.label | j | -| test.cpp:335:22:335:23 | scanf output argument | semmle.label | scanf output argument | -| test.cpp:337:8:337:8 | i | semmle.label | i | -| test.cpp:344:23:344:24 | scanf output argument | semmle.label | scanf output argument | -| test.cpp:346:8:346:8 | i | semmle.label | i | -| test.cpp:353:26:353:27 | scanf output argument | semmle.label | scanf output argument | -| test.cpp:353:30:353:31 | scanf output argument | semmle.label | scanf output argument | -| test.cpp:354:8:354:8 | d | semmle.label | d | -| test.cpp:355:8:355:8 | n | semmle.label | n | -| test.cpp:362:62:362:63 | sscanf output argument | semmle.label | sscanf output argument | -| test.cpp:364:17:364:17 | n | semmle.label | n | | test.cpp:403:29:403:30 | sscanf output argument | semmle.label | sscanf output argument | | test.cpp:404:18:404:25 | u | semmle.label | u | -| test.cpp:413:19:413:20 | scanf output argument | semmle.label | scanf output argument | -| test.cpp:416:7:416:7 | i | semmle.label | i | -| test.cpp:420:19:420:20 | scanf output argument | semmle.label | scanf output argument | -| test.cpp:423:7:423:7 | i | semmle.label | i | | test.cpp:455:41:455:46 | sscanf output argument | semmle.label | sscanf output argument | | test.cpp:460:6:460:10 | value | semmle.label | value | | test.cpp:467:20:467:25 | scanf output argument | semmle.label | scanf output argument | @@ -136,10 +149,6 @@ nodes | test.cpp:484:9:484:9 | i | semmle.label | i | | test.cpp:491:25:491:26 | scanf output argument | semmle.label | scanf output argument | | test.cpp:495:8:495:8 | i | semmle.label | i | -| test.cpp:501:25:501:26 | scanf output argument | semmle.label | scanf output argument | -| test.cpp:505:9:505:9 | i | semmle.label | i | -| test.cpp:512:25:512:26 | scanf output argument | semmle.label | scanf output argument | -| test.cpp:516:9:516:9 | i | semmle.label | i | | test.cpp:525:35:525:36 | sscanf output argument | semmle.label | sscanf output argument | | test.cpp:525:39:525:40 | sscanf output argument | semmle.label | sscanf output argument | | test.cpp:525:43:525:44 | sscanf output argument | semmle.label | sscanf output argument | @@ -157,32 +166,4 @@ nodes | test.cpp:548:8:548:8 | d | semmle.label | d | | test.cpp:549:8:549:8 | e | semmle.label | e | | test.cpp:550:8:550:8 | f | semmle.label | f | -| test.cpp:559:30:559:31 | scanf output argument | semmle.label | scanf output argument | -| test.cpp:561:9:561:9 | i | semmle.label | i | -| test.cpp:567:35:567:36 | scanf output argument | semmle.label | scanf output argument | -| test.cpp:569:9:569:9 | i | semmle.label | i | -| test.cpp:575:30:575:31 | scanf output argument | semmle.label | scanf output argument | -| test.cpp:577:9:577:9 | i | semmle.label | i | subpaths -#select -| 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 | -| 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 | -| 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 | -| 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 | -| 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 | -| 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 | -| 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 | -| 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 | -| 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 | -| 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 | -| 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 | -| 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 | -| 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 | -| 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 | -| 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 | -| 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 | -| 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 | -| 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 | -| 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 | -| 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 | -| 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 | diff --git a/cpp/ql/test/query-tests/Critical/MissingCheckScanf/overwrites.c b/cpp/ql/test/query-tests/Critical/MissingCheckScanf/overwrites.c new file mode 100644 index 000000000000..f0bb0a2f1590 --- /dev/null +++ b/cpp/ql/test/query-tests/Critical/MissingCheckScanf/overwrites.c @@ -0,0 +1,181 @@ +typedef struct File FILE; + +int scanf(const char *format, ...); +int fscanf(FILE *stream, const char *format, ...); +int sscanf(const char *input, const char *format, ...); +void finish_reading(FILE *stream); +void use(int value); +int maybe(void); + +// Independently written regression for github/codeql#12412. +int read_sample(FILE *stream, int fallback) { + int sample; + int conversions = fscanf(stream, "%d", &sample); + if (conversions < 1) { + sample = fallback; + } + finish_reading(stream); + return sample; // GOOD +} + +void overwrite_with_parameter(int fallback) { + int i; + if (scanf("%d", &i) < 1) { + i = fallback; + } + use(i); // GOOD +} + +void saved_return_value(void) { + int i; + int result = scanf("%d", &i); + if (result != 1) { + i = 0; + } + use(i); // GOOD +} + +void successful_branch(void) { + int i; + if (scanf("%d", &i) >= 1) { + use(i); // GOOD + } else { + i = 0; + } + use(i); // GOOD +} + +void overwrite_both_outputs(const char *input) { + int i, j; + if (sscanf(input, "%d %d", &i, &j) != 2) { + i = 0; + j = 0; + } + use(i); // GOOD + use(j); // GOOD +} + +void insufficient_conversion_count(void) { + int i, j; + if (scanf("%d %d", &i, &j) < 1) { // $ Source[cpp/missing-check-scanf] + i = 0; + j = 0; + } + use(i); // GOOD + use(j); // $ Alert[cpp/missing-check-scanf] +} + +void suppressed_conversion(const char *input) { + int i; + if (sscanf(input, "%*d %d", &i) != 1) { + i = 0; + } + use(i); // GOOD +} + +void count_conversion(const char *input) { + int i, count; + if (sscanf(input, "%d %n", &i, &count) != 1) { + i = 0; + count = 0; + } + use(i); // GOOD + use(count); // GOOD +} + +void conditional_overwrite(void) { + int i; + if (scanf("%d", &i) != 1) { // $ Source[cpp/missing-check-scanf] + if (maybe()) { + i = 0; + } + } + use(i); // $ Alert[cpp/missing-check-scanf] +} + +void overwrite_other_variable(void) { + int i, j = 1; + if (scanf("%d", &i) != 1) { // $ Source[cpp/missing-check-scanf] + j = 0; + } + use(i); // $ Alert[cpp/missing-check-scanf] +} + +void dependent_overwrite(void) { + int i; + if (scanf("%d", &i) != 1) { // $ Source[cpp/missing-check-scanf] + i = i + 1; // $ Alert[cpp/missing-check-scanf] + } + use(i); // GOOD: the unsafe read was reported above +} + +void increment_on_failure(void) { + int i; + if (scanf("%d", &i) != 1) { // $ Source[cpp/missing-check-scanf] + ++i; // $ Alert[cpp/missing-check-scanf] + } + use(i); // GOOD: the unsafe read was reported above +} + +void alias_read_on_failure(void) { + int i; + int *alias = &i; + if (scanf("%d", &i) != 1) { // $ Source[cpp/missing-check-scanf] + i = *alias + 1; // $ MISSING: Alert[cpp/missing-check-scanf] + } + use(i); // $ Alert[cpp/missing-check-scanf] +} + +void read_before_overwrite(void) { + int i; + if (scanf("%d", &i) != 1) { // $ Source[cpp/missing-check-scanf] + use(i); // $ Alert[cpp/missing-check-scanf] + i = 0; + } + use(i); // GOOD +} + +void check_other_output(void) { + int i, j; + scanf("%d", &i); // $ Source[cpp/missing-check-scanf] + if (scanf("%d", &j) == 1) { + use(i); // $ Alert[cpp/missing-check-scanf] + } +} + +void unchecked_call_then_overwrite(void) { + int i; + scanf("%d", &i); // $ Source[cpp/missing-check-scanf] + use(i); // $ Alert[cpp/missing-check-scanf] + if (scanf("%d", &i) != 1) { + i = 0; + } + use(i); // GOOD +} + +void overwritten_return_value(void) { + int i; + int result = scanf("%d", &i); // $ Source[cpp/missing-check-scanf] + result = 1; + if (result != 1) { + i = 0; + } + use(i); // $ Alert[cpp/missing-check-scanf] +} + +void only_one_output_is_overwritten(void) { + int i, j; + if (scanf("%d %d", &i, &j) != 2) { // $ Source[cpp/missing-check-scanf] + i = 0; + } + use(i); // GOOD + use(j); // $ Alert[cpp/missing-check-scanf] +} + +void conditional_read_in_overwrite(void) { + int i; + if (scanf("%d", &i) != 1) { // $ Source[cpp/missing-check-scanf] + i = maybe() ? i : 0; // $ Alert[cpp/missing-check-scanf] + } + use(i); // GOOD: the unsafe read was reported above +} diff --git a/cpp/ql/test/query-tests/Critical/MissingCheckScanf/overwrites.cpp b/cpp/ql/test/query-tests/Critical/MissingCheckScanf/overwrites.cpp new file mode 100644 index 000000000000..996021b93c92 --- /dev/null +++ b/cpp/ql/test/query-tests/Critical/MissingCheckScanf/overwrites.cpp @@ -0,0 +1,49 @@ +int scanf(const char *format, ...); +void use(int value); + +void reference_read_on_failure() { + int value; + int &alias = value; + if (scanf("%d", &value) != 1) { // $ Source[cpp/missing-check-scanf] + value = alias + 1; // $ MISSING: Alert[cpp/missing-check-scanf] + } + use(value); // $ Alert[cpp/missing-check-scanf] +} + +void reference_with_safe_overwrite() { + int value; + int &alias = value; + if (scanf("%d", &value) != 1) { // $ Source[cpp/missing-check-scanf] + value = 0; + } + use(value); // $ SPURIOUS: Alert[cpp/missing-check-scanf] +} + +void scanf_through_alias() { + int value; + int *alias = &value; + if (scanf("%d", alias) != 1) { + value = 0; + } + use(value); // GOOD +} + +void reference_output_with_unsafe_alias() { + int value; + int &output = value; + int *alias = &value; + if (scanf("%d", &output) != 1) { // $ Source[cpp/missing-check-scanf] + output = *alias + 1; // $ MISSING: Alert[cpp/missing-check-scanf] + } + use(output); // $ Alert[cpp/missing-check-scanf] +} + +void typedef_reference_output_with_safe_overwrite() { + typedef int &Output; + int value; + Output output = value; + if (scanf("%d", &output) != 1) { // $ Source[cpp/missing-check-scanf] + output = 0; + } + use(output); // $ SPURIOUS: Alert[cpp/missing-check-scanf] +} diff --git a/cpp/ql/test/query-tests/Critical/MissingCheckScanf/test.cpp b/cpp/ql/test/query-tests/Critical/MissingCheckScanf/test.cpp index 25f04f1938bd..c650f7d35b79 100644 --- a/cpp/ql/test/query-tests/Critical/MissingCheckScanf/test.cpp +++ b/cpp/ql/test/query-tests/Critical/MissingCheckScanf/test.cpp @@ -410,17 +410,17 @@ char *my_string_copy() { void scan_and_write() { { int i; - if (scanf("%d", &i) < 1) { // $ Source[cpp/missing-check-scanf] + if (scanf("%d", &i) < 1) { i = 0; } - use(i); // $ SPURIOUS: Alert[cpp/missing-check-scanf] // GOOD [FALSE POSITIVE]: variable is overwritten with a default value when scanf fails + use(i); // GOOD: variable is overwritten with a default value when scanf fails } { int i; - if (scanf("%d", &i) != 1) { // $ Source[cpp/missing-check-scanf] + if (scanf("%d", &i) != 1) { i = 0; } - use(i); // $ SPURIOUS: Alert[cpp/missing-check-scanf] // GOOD [FALSE POSITIVE]: variable is overwritten with a default value when scanf fails + use(i); // GOOD: variable is overwritten with a default value when scanf fails } }