From 2902a19a50ddb5a71eeb32d12fe2cb1366589bda Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Thu, 14 May 2026 18:37:38 +0100 Subject: [PATCH 1/5] C++: Add more scanf testing. --- .../source-sink-tests/sources-and-sinks.cpp | 38 +++++++++++++++++++ .../scanf/scanfFormatLiteral.expected | 10 ++--- .../scanf/scanfFunctionCall.expected | 8 ++-- .../scanf/scanfFunctionCallOutput.expected | 6 +++ .../scanf/scanfFunctionCallOutput.ql | 5 +++ cpp/ql/test/library-tests/scanf/test.c | 4 +- 6 files changed, 61 insertions(+), 10 deletions(-) create mode 100644 cpp/ql/test/library-tests/scanf/scanfFunctionCallOutput.expected create mode 100644 cpp/ql/test/library-tests/scanf/scanfFunctionCallOutput.ql diff --git a/cpp/ql/test/library-tests/dataflow/source-sink-tests/sources-and-sinks.cpp b/cpp/ql/test/library-tests/dataflow/source-sink-tests/sources-and-sinks.cpp index e4947a112f8d..7757eb46dd31 100644 --- a/cpp/ql/test/library-tests/dataflow/source-sink-tests/sources-and-sinks.cpp +++ b/cpp/ql/test/library-tests/dataflow/source-sink-tests/sources-and-sinks.cpp @@ -131,3 +131,41 @@ void test_strsafe_gets() { StringCchGetsExA(dest, sizeof(dest), &end, &remaining, 0); // $ local_source } } + +int scanf_s(const char *format, ...); +int fscanf_s(FILE *stream, const char *format, ...); + +void test_scanf_s(FILE *stream) { + { + int n1, n2; + scanf_s( + "%d", + &n1, // $ MISSING: local_source + &n2); // $ MISSING: local_source + } + + { + int n; + fscanf_s(stream, "%d", &n); // $ MISSING: remote_source + } + + { + int n1, n2; + char buf[256]; + scanf_s("%d %s", + &n1, // $ MISSING: local_source + buf, // $ MISSING: local_source + 256, + &n2); // $ MISSING: local_source + } + + { + int n1, n2; + char buf[256]; + fscanf_s(stream, "%d %s", + &n1, // $ MISSING: remote_source + buf, // $ MISSING: remote_source + 256, + &n2); // $ MISSING: remote_source + } +} \ No newline at end of file diff --git a/cpp/ql/test/library-tests/scanf/scanfFormatLiteral.expected b/cpp/ql/test/library-tests/scanf/scanfFormatLiteral.expected index 263ea54b8cd3..9fcbb42a2166 100644 --- a/cpp/ql/test/library-tests/scanf/scanfFormatLiteral.expected +++ b/cpp/ql/test/library-tests/scanf/scanfFormatLiteral.expected @@ -1,5 +1,5 @@ -| test.c:18:2:18:6 | call to scanf | 0 | s | 0 | 0 | -| test.c:19:2:19:7 | call to fscanf | 0 | s | 10 | 10 | -| test.c:19:2:19:7 | call to fscanf | 1 | i | 0 | 0 | -| test.c:20:2:20:7 | call to sscanf | 0 | s | 0 | 0 | -| test.c:21:2:21:8 | call to swscanf | 0 | s | 10 | 10 | +| test.c:19:2:19:6 | call to scanf | 0 | s | 0 | 0 | +| test.c:20:2:20:7 | call to fscanf | 0 | s | 10 | 10 | +| test.c:20:2:20:7 | call to fscanf | 1 | i | 0 | 0 | +| test.c:21:2:21:7 | call to sscanf | 0 | s | 0 | 0 | +| test.c:22:2:22:8 | call to swscanf | 0 | s | 10 | 10 | diff --git a/cpp/ql/test/library-tests/scanf/scanfFunctionCall.expected b/cpp/ql/test/library-tests/scanf/scanfFunctionCall.expected index b0dce385b7bb..7e74a349f8bd 100644 --- a/cpp/ql/test/library-tests/scanf/scanfFunctionCall.expected +++ b/cpp/ql/test/library-tests/scanf/scanfFunctionCall.expected @@ -1,5 +1,5 @@ | ms.cpp:17:3:17:8 | call to sscanf | 0 | 1 | ms.cpp:17:24:17:30 | %I64i | non-wide | -| test.c:18:2:18:6 | call to scanf | 0 | 0 | test.c:18:8:18:11 | %s | non-wide | -| test.c:19:2:19:7 | call to fscanf | 0 | 1 | test.c:19:15:19:23 | %10s %i | non-wide | -| test.c:20:2:20:7 | call to sscanf | 0 | 1 | test.c:20:19:20:28 | %*i%s%*s | non-wide | -| test.c:21:2:21:8 | call to swscanf | 0 | 1 | test.c:21:21:21:26 | %10s | wide | +| test.c:19:2:19:6 | call to scanf | 0 | 0 | test.c:19:8:19:11 | %s | non-wide | +| test.c:20:2:20:7 | call to fscanf | 0 | 1 | test.c:20:15:20:23 | %10s %i | non-wide | +| test.c:21:2:21:7 | call to sscanf | 0 | 1 | test.c:21:19:21:28 | %*i%s%*s | non-wide | +| test.c:22:2:22:8 | call to swscanf | 0 | 1 | test.c:22:21:22:26 | %10s | wide | diff --git a/cpp/ql/test/library-tests/scanf/scanfFunctionCallOutput.expected b/cpp/ql/test/library-tests/scanf/scanfFunctionCallOutput.expected new file mode 100644 index 000000000000..f232db160da6 --- /dev/null +++ b/cpp/ql/test/library-tests/scanf/scanfFunctionCallOutput.expected @@ -0,0 +1,6 @@ +| ms.cpp:17:3:17:8 | call to sscanf | ms.cpp:17:33:17:36 | & ... | 0 | +| test.c:19:2:19:6 | call to scanf | test.c:19:14:19:19 | buffer | 0 | +| test.c:20:2:20:7 | call to fscanf | test.c:20:26:20:31 | buffer | 0 | +| test.c:20:2:20:7 | call to fscanf | test.c:20:34:20:34 | i | 1 | +| test.c:21:2:21:7 | call to sscanf | test.c:21:31:21:36 | buffer | 0 | +| test.c:22:2:22:8 | call to swscanf | test.c:22:29:22:35 | wbuffer | 0 | diff --git a/cpp/ql/test/library-tests/scanf/scanfFunctionCallOutput.ql b/cpp/ql/test/library-tests/scanf/scanfFunctionCallOutput.ql new file mode 100644 index 000000000000..a3d40604cfa8 --- /dev/null +++ b/cpp/ql/test/library-tests/scanf/scanfFunctionCallOutput.ql @@ -0,0 +1,5 @@ +import semmle.code.cpp.commons.Scanf + +from ScanfFunctionCall sfc, Expr e, int n +where e = sfc.getOutputArgument(n) +select sfc, e, n diff --git a/cpp/ql/test/library-tests/scanf/test.c b/cpp/ql/test/library-tests/scanf/test.c index c378eec72220..eb99bbec7adf 100644 --- a/cpp/ql/test/library-tests/scanf/test.c +++ b/cpp/ql/test/library-tests/scanf/test.c @@ -7,18 +7,20 @@ int scanf(const char *format, ...); int fscanf(FILE *stream, const char *format, ...); int sscanf(const char *s, const char *format, ...); int swscanf(const wchar_t* ws, const wchar_t* format, ...); +int scanf_s(const char *format, ...); int main(int argc, char *argv[]) { char buffer[256]; wchar_t wbuffer[256]; FILE *file; - int i; + int i, i2; scanf("%s", buffer); fscanf(file, "%10s %i", buffer, i); sscanf("Hello.", "%*i%s%*s", buffer); swscanf(L"Hello.", "%10s", wbuffer); + scanf_s("%d %s %d", &i, buffer, 10, &i2); return 0; } \ No newline at end of file From 16235d7acaefbca3b811490b9b061e58fb25c08e Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Mon, 18 May 2026 13:20:31 +0100 Subject: [PATCH 2/5] C++: Add a 'call' column to 'hasRemoteFlowSource' and 'hasLocalFlowSource' to support modeling of 'scanf_s'. --- .../code/cpp/models/implementations/Scanf.qll | 22 ++++++++++++----- .../code/cpp/models/interfaces/FlowSource.qll | 24 +++++++++++++++++-- .../semmle/code/cpp/security/FlowSources.qll | 5 ++-- .../CWE/CWE-020/ExternalAPIsSpecific.qll | 5 +--- .../CWE/CWE-311/CleartextTransmission.ql | 3 +-- 5 files changed, 42 insertions(+), 17 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/models/implementations/Scanf.qll b/cpp/ql/lib/semmle/code/cpp/models/implementations/Scanf.qll index f1b3edbe3370..d7efc44afb58 100644 --- a/cpp/ql/lib/semmle/code/cpp/models/implementations/Scanf.qll +++ b/cpp/ql/lib/semmle/code/cpp/models/implementations/Scanf.qll @@ -69,13 +69,24 @@ abstract private class ScanfFunctionModel extends ArrayFunction, TaintFunction, } } +private predicate hasFlowSource( + ScanfFunction func, ScanfFunctionCall call, FunctionOutput output, string description +) { + exists(int n, Expr arg | + call.getScanfFunction() = func and + call.getOutputArgument(_) = arg and + call.getArgument(n) = arg and + output.isParameterDeref(n) and + description = "value read by " + func.getName() + ) +} + /** * The standard function `scanf` and its assorted variants */ private class ScanfModel extends ScanfFunctionModel, LocalFlowSourceFunction instanceof Scanf { - override predicate hasLocalFlowSource(FunctionOutput output, string description) { - output.isParameterDeref(any(int i | i >= this.getArgsStartPosition())) and - description = "value read by " + this.getName() + override predicate hasLocalFlowSource(Call call, FunctionOutput output, string description) { + hasFlowSource(this, call, output, description) } } @@ -83,9 +94,8 @@ private class ScanfModel extends ScanfFunctionModel, LocalFlowSourceFunction ins * The standard function `fscanf` and its assorted variants */ private class FscanfModel extends ScanfFunctionModel, RemoteFlowSourceFunction instanceof Fscanf { - override predicate hasRemoteFlowSource(FunctionOutput output, string description) { - output.isParameterDeref(any(int i | i >= this.getArgsStartPosition())) and - description = "value read by " + this.getName() + override predicate hasRemoteFlowSource(Call call, FunctionOutput output, string description) { + hasFlowSource(this, call, output, description) } override predicate hasSocketInput(FunctionInput input) { diff --git a/cpp/ql/lib/semmle/code/cpp/models/interfaces/FlowSource.qll b/cpp/ql/lib/semmle/code/cpp/models/interfaces/FlowSource.qll index d2103f83bc0e..cf28fd0d6d30 100644 --- a/cpp/ql/lib/semmle/code/cpp/models/interfaces/FlowSource.qll +++ b/cpp/ql/lib/semmle/code/cpp/models/interfaces/FlowSource.qll @@ -18,7 +18,17 @@ abstract class RemoteFlowSourceFunction extends Function { /** * Holds if remote data described by `description` flows from `output` of a call to this function. */ - abstract predicate hasRemoteFlowSource(FunctionOutput output, string description); + predicate hasRemoteFlowSource(FunctionOutput output, string description) { + this.hasRemoteFlowSource(_, output, description) + } + + /** + * Holds if remote data described by `description` flows from `output` of `call` to this function. + */ + predicate hasRemoteFlowSource(Call call, FunctionOutput output, string description) { + call.getTarget() = this and + this.hasRemoteFlowSource(output, description) + } /** * Holds if remote data from this source comes from a socket or stream @@ -35,7 +45,17 @@ abstract class LocalFlowSourceFunction extends Function { /** * Holds if data described by `description` flows from `output` of a call to this function. */ - abstract predicate hasLocalFlowSource(FunctionOutput output, string description); + predicate hasLocalFlowSource(FunctionOutput output, string description) { + this.hasLocalFlowSource(_, output, description) + } + + /** + * Holds if data described by `description` flows from `output` of `call` to this function. + */ + predicate hasLocalFlowSource(Call call, FunctionOutput output, string description) { + call.getTarget() = this and + this.hasLocalFlowSource(output, description) + } } /** A library function that sends data over a network connection. */ diff --git a/cpp/ql/lib/semmle/code/cpp/security/FlowSources.qll b/cpp/ql/lib/semmle/code/cpp/security/FlowSources.qll index eba6f9339ffe..33695fdd51ab 100644 --- a/cpp/ql/lib/semmle/code/cpp/security/FlowSources.qll +++ b/cpp/ql/lib/semmle/code/cpp/security/FlowSources.qll @@ -28,8 +28,7 @@ private class RemoteModelSource extends RemoteFlowSource { RemoteModelSource() { exists(CallInstruction call, RemoteFlowSourceFunction func, FunctionOutput output | - call.getStaticCallTarget() = func and - func.hasRemoteFlowSource(output, sourceType) and + func.hasRemoteFlowSource(call.getConvertedResultExpression(), output, sourceType) and this = callOutput(call, output) ) } @@ -46,7 +45,7 @@ private class LocalModelSource extends LocalFlowSource { LocalModelSource() { exists(CallInstruction call, LocalFlowSourceFunction func, FunctionOutput output | call.getStaticCallTarget() = func and - func.hasLocalFlowSource(output, sourceType) and + func.hasLocalFlowSource(call.getConvertedResultExpression(), output, sourceType) and this = callOutput(call, output) ) } diff --git a/cpp/ql/src/Security/CWE/CWE-020/ExternalAPIsSpecific.qll b/cpp/ql/src/Security/CWE/CWE-020/ExternalAPIsSpecific.qll index f0876800874c..f98b295cb74e 100644 --- a/cpp/ql/src/Security/CWE/CWE-020/ExternalAPIsSpecific.qll +++ b/cpp/ql/src/Security/CWE/CWE-020/ExternalAPIsSpecific.qll @@ -44,10 +44,7 @@ class ExternalApiDataNode extends DataFlow::Node { /** A configuration for tracking flow from `RemoteFlowSource`s to `ExternalApiDataNode`s. */ private module UntrustedDataToExternalApiConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { - exists(RemoteFlowSourceFunction remoteFlow | - remoteFlow = source.asExpr().(Call).getTarget() and - remoteFlow.hasRemoteFlowSource(_, _) - ) + any(RemoteFlowSourceFunction remoteFlow).hasRemoteFlowSource(source.asExpr(), _, _) } predicate isSink(DataFlow::Node sink) { sink instanceof ExternalApiDataNode } diff --git a/cpp/ql/src/Security/CWE/CWE-311/CleartextTransmission.ql b/cpp/ql/src/Security/CWE/CWE-311/CleartextTransmission.ql index 392650022e20..207aec189b2e 100644 --- a/cpp/ql/src/Security/CWE/CWE-311/CleartextTransmission.ql +++ b/cpp/ql/src/Security/CWE/CWE-311/CleartextTransmission.ql @@ -94,9 +94,8 @@ class Recv extends SendRecv instanceof RemoteFlowSourceFunction { } override Expr getDataExpr(Call call) { - call.getTarget() = this and exists(FunctionOutput output, int arg | - super.hasRemoteFlowSource(output, _) and + super.hasRemoteFlowSource(call, output, _) and output.isParameterDeref(arg) and result = call.getArgument(arg) ) From 5add24be59bd1f35d33a304da0aad5f553f8f3e3 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Thu, 14 May 2026 18:38:34 +0100 Subject: [PATCH 3/5] C++: Add scanf_s models. --- cpp/ql/lib/semmle/code/cpp/commons/Scanf.qll | 8 +++++++- .../source-sink-tests/sources-and-sinks.cpp | 18 +++++++++--------- .../scanf/scanfFormatLiteral.expected | 3 +++ .../scanf/scanfFunctionCall.expected | 1 + .../scanf/scanfFunctionCallOutput.expected | 4 ++++ 5 files changed, 24 insertions(+), 10 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/commons/Scanf.qll b/cpp/ql/lib/semmle/code/cpp/commons/Scanf.qll index 98280a522cfd..b5d6bcc9de80 100644 --- a/cpp/ql/lib/semmle/code/cpp/commons/Scanf.qll +++ b/cpp/ql/lib/semmle/code/cpp/commons/Scanf.qll @@ -34,6 +34,7 @@ class Scanf extends ScanfFunction instanceof TopLevelFunction { Scanf() { this.hasGlobalOrStdOrBslName("scanf") or // scanf(format, args...) this.hasGlobalOrStdOrBslName("wscanf") or // wscanf(format, args...) + this.hasGlobalOrStdOrBslName("scanf_s") or // scanf_s(format, args...) this.hasGlobalName("_scanf_l") or // _scanf_l(format, locale, args...) this.hasGlobalName("_wscanf_l") } @@ -50,6 +51,7 @@ class Fscanf extends ScanfFunction instanceof TopLevelFunction { Fscanf() { this.hasGlobalOrStdOrBslName("fscanf") or // fscanf(src_stream, format, args...) this.hasGlobalOrStdOrBslName("fwscanf") or // fwscanf(src_stream, format, args...) + this.hasGlobalOrStdOrBslName("fscanf_s") or // fscanf_s(src_stream, format, args...) this.hasGlobalName("_fscanf_l") or // _fscanf_l(src_stream, format, locale, args...) this.hasGlobalName("_fwscanf_l") } @@ -66,8 +68,12 @@ class Sscanf extends ScanfFunction instanceof TopLevelFunction { Sscanf() { this.hasGlobalOrStdOrBslName("sscanf") or // sscanf(src_stream, format, args...) this.hasGlobalOrStdOrBslName("swscanf") or // swscanf(src, format, args...) + this.hasGlobalOrStdOrBslName("sscanf_s") or // sscanf_s(src, format, args...) + this.hasGlobalOrStdOrBslName("swscanf_s") or // swscanf_s(src, format, args...) this.hasGlobalName("_sscanf_l") or // _sscanf_l(src, format, locale, args...) - this.hasGlobalName("_swscanf_l") + this.hasGlobalName("_swscanf_l") or // _swscanf_l(src, format, locale, args...) + this.hasGlobalName("_sscanf_s_l") or // _sscanf_s_l(src, format, locale, args...) + this.hasGlobalName("_swscanf_s_l") // _swscanf_s_l(src, format, locale, args...) } override int getInputParameterIndex() { result = 0 } diff --git a/cpp/ql/test/library-tests/dataflow/source-sink-tests/sources-and-sinks.cpp b/cpp/ql/test/library-tests/dataflow/source-sink-tests/sources-and-sinks.cpp index 7757eb46dd31..bca2dd136c13 100644 --- a/cpp/ql/test/library-tests/dataflow/source-sink-tests/sources-and-sinks.cpp +++ b/cpp/ql/test/library-tests/dataflow/source-sink-tests/sources-and-sinks.cpp @@ -140,32 +140,32 @@ void test_scanf_s(FILE *stream) { int n1, n2; scanf_s( "%d", - &n1, // $ MISSING: local_source - &n2); // $ MISSING: local_source + &n1, // $ local_source + &n2); // $ local_source } { int n; - fscanf_s(stream, "%d", &n); // $ MISSING: remote_source + fscanf_s(stream, "%d", &n); // $ remote_source } { int n1, n2; char buf[256]; scanf_s("%d %s", - &n1, // $ MISSING: local_source - buf, // $ MISSING: local_source + &n1, // $ local_source + buf, // $ local_source 256, - &n2); // $ MISSING: local_source + &n2); // $ local_source } { int n1, n2; char buf[256]; fscanf_s(stream, "%d %s", - &n1, // $ MISSING: remote_source - buf, // $ MISSING: remote_source + &n1, // $ remote_source + buf, // $ remote_source 256, - &n2); // $ MISSING: remote_source + &n2); // $ remote_source } } \ No newline at end of file diff --git a/cpp/ql/test/library-tests/scanf/scanfFormatLiteral.expected b/cpp/ql/test/library-tests/scanf/scanfFormatLiteral.expected index 9fcbb42a2166..f6fc707b4945 100644 --- a/cpp/ql/test/library-tests/scanf/scanfFormatLiteral.expected +++ b/cpp/ql/test/library-tests/scanf/scanfFormatLiteral.expected @@ -3,3 +3,6 @@ | test.c:20:2:20:7 | call to fscanf | 1 | i | 0 | 0 | | test.c:21:2:21:7 | call to sscanf | 0 | s | 0 | 0 | | test.c:22:2:22:8 | call to swscanf | 0 | s | 10 | 10 | +| test.c:23:2:23:8 | call to scanf_s | 0 | d | 0 | 0 | +| test.c:23:2:23:8 | call to scanf_s | 1 | s | 0 | 0 | +| test.c:23:2:23:8 | call to scanf_s | 2 | d | 0 | 0 | diff --git a/cpp/ql/test/library-tests/scanf/scanfFunctionCall.expected b/cpp/ql/test/library-tests/scanf/scanfFunctionCall.expected index 7e74a349f8bd..ba658bd6d2f5 100644 --- a/cpp/ql/test/library-tests/scanf/scanfFunctionCall.expected +++ b/cpp/ql/test/library-tests/scanf/scanfFunctionCall.expected @@ -3,3 +3,4 @@ | test.c:20:2:20:7 | call to fscanf | 0 | 1 | test.c:20:15:20:23 | %10s %i | non-wide | | test.c:21:2:21:7 | call to sscanf | 0 | 1 | test.c:21:19:21:28 | %*i%s%*s | non-wide | | test.c:22:2:22:8 | call to swscanf | 0 | 1 | test.c:22:21:22:26 | %10s | wide | +| test.c:23:2:23:8 | call to scanf_s | 0 | 0 | test.c:23:10:23:19 | %d %s %d | non-wide | diff --git a/cpp/ql/test/library-tests/scanf/scanfFunctionCallOutput.expected b/cpp/ql/test/library-tests/scanf/scanfFunctionCallOutput.expected index f232db160da6..2efb7094a33c 100644 --- a/cpp/ql/test/library-tests/scanf/scanfFunctionCallOutput.expected +++ b/cpp/ql/test/library-tests/scanf/scanfFunctionCallOutput.expected @@ -4,3 +4,7 @@ | test.c:20:2:20:7 | call to fscanf | test.c:20:34:20:34 | i | 1 | | test.c:21:2:21:7 | call to sscanf | test.c:21:31:21:36 | buffer | 0 | | test.c:22:2:22:8 | call to swscanf | test.c:22:29:22:35 | wbuffer | 0 | +| test.c:23:2:23:8 | call to scanf_s | test.c:23:22:23:23 | & ... | 0 | +| test.c:23:2:23:8 | call to scanf_s | test.c:23:26:23:31 | buffer | 1 | +| test.c:23:2:23:8 | call to scanf_s | test.c:23:34:23:35 | 10 | 2 | +| test.c:23:2:23:8 | call to scanf_s | test.c:23:38:23:40 | & ... | 3 | From 5f10a882085bf93489c86929a5e423f2692f98a0 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Thu, 14 May 2026 19:24:59 +0100 Subject: [PATCH 4/5] C++: Handle size arguments in 'getOutputArgument'. --- cpp/ql/lib/semmle/code/cpp/commons/Scanf.qll | 47 ++++++++++++++++++- .../scanf/scanfFunctionCallOutput.expected | 3 +- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/commons/Scanf.qll b/cpp/ql/lib/semmle/code/cpp/commons/Scanf.qll index b5d6bcc9de80..6ae59eac23e8 100644 --- a/cpp/ql/lib/semmle/code/cpp/commons/Scanf.qll +++ b/cpp/ql/lib/semmle/code/cpp/commons/Scanf.qll @@ -25,6 +25,15 @@ abstract class ScanfFunction extends Function { * (rather than a `char*`). */ predicate isWideCharDefault() { exists(this.getName().indexOf("wscanf")) } + + /** Holds if this is one of the `scanf_s` variants. */ + predicate isSVariant() { + exists(string name | name = this.getName() | + name.matches("%\\_s") + or + name.matches("%\\_s\\_l") + ) + } } /** @@ -103,6 +112,14 @@ class Snscanf extends ScanfFunction instanceof TopLevelFunction { int getInputLengthParameterIndex() { result = 1 } } +private predicate isCharLike(Type t) { t instanceof CharType or t instanceof Wchar_t } + +private predicate isStringLike(Type t) { + isCharLike(t.(PointerType).getBaseType()) + or + isCharLike(t.(ArrayType).getBaseType()) +} + /** * A call to one of the `scanf` functions. */ @@ -136,14 +153,40 @@ class ScanfFunctionCall extends FunctionCall { */ predicate isWideCharDefault() { this.getScanfFunction().isWideCharDefault() } + bindingset[this, k] + pragma[inline_late] + private predicate isSizeArgument(int k) { + // The first vararg is never the size argument since a size argument must + // always follow a string buffer argument. + k > 0 and + isStringLike(this.getArgument(this.getScanfFunction().getNumberOfParameters() + k - 1) + .getUnspecifiedType()) + } + /** * Gets the output argument at position `n` in the vararg list of this call. * * The range of `n` is from `0` to `this.getNumberOfOutputArguments() - 1`. */ Expr getOutputArgument(int n) { - result = this.getArgument(this.getTarget().getNumberOfParameters() + n) and - n >= 0 + exists(ScanfFunction target | target = this.getScanfFunction() | + // If this is an S variant then every string buffer argument has a + // corresponding size argument immediately following it, so we need to + // skip over those size arguments when counting the output arguments. + if target.isSVariant() + then + result = + rank[n + 1](Expr arg, int k | + k >= 0 and + arg = this.getArgument(target.getNumberOfParameters() + k) and + not this.isSizeArgument(k) + | + arg order by k + ) + else ( + n >= 0 and result = this.getArgument(target.getNumberOfParameters() + n) + ) + ) } /** diff --git a/cpp/ql/test/library-tests/scanf/scanfFunctionCallOutput.expected b/cpp/ql/test/library-tests/scanf/scanfFunctionCallOutput.expected index 2efb7094a33c..87998b4c367b 100644 --- a/cpp/ql/test/library-tests/scanf/scanfFunctionCallOutput.expected +++ b/cpp/ql/test/library-tests/scanf/scanfFunctionCallOutput.expected @@ -6,5 +6,4 @@ | test.c:22:2:22:8 | call to swscanf | test.c:22:29:22:35 | wbuffer | 0 | | test.c:23:2:23:8 | call to scanf_s | test.c:23:22:23:23 | & ... | 0 | | test.c:23:2:23:8 | call to scanf_s | test.c:23:26:23:31 | buffer | 1 | -| test.c:23:2:23:8 | call to scanf_s | test.c:23:34:23:35 | 10 | 2 | -| test.c:23:2:23:8 | call to scanf_s | test.c:23:38:23:40 | & ... | 3 | +| test.c:23:2:23:8 | call to scanf_s | test.c:23:38:23:40 | & ... | 2 | From 19781e53e710aa07a644543d76879af8c5752d2e Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Mon, 18 May 2026 13:24:59 +0100 Subject: [PATCH 5/5] C++: Add change notes. --- cpp/ql/lib/change-notes/2026-05-15-secure-scanf.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 cpp/ql/lib/change-notes/2026-05-15-secure-scanf.md diff --git a/cpp/ql/lib/change-notes/2026-05-15-secure-scanf.md b/cpp/ql/lib/change-notes/2026-05-15-secure-scanf.md new file mode 100644 index 000000000000..0b8d5a79a725 --- /dev/null +++ b/cpp/ql/lib/change-notes/2026-05-15-secure-scanf.md @@ -0,0 +1,5 @@ +--- +category: minorAnalysis +--- +* Added flow source models for `scanf_s` and related functions. +* Added a `Call` column to `LocalFlowSourceFunction::hasLocalFlowSource` and `RemoteFlowSourceFunction::hasRemoteFlowSource`. The old predicates without a `Call` column continue to be supported. \ No newline at end of file