Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions cpp/ql/lib/change-notes/2026-05-15-secure-scanf.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Added flow source models for `scanf_s` and related functions.
55 changes: 52 additions & 3 deletions cpp/ql/lib/semmle/code/cpp/commons/Scanf.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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")
)
}
}

/**
Expand All @@ -34,6 +43,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")
}
Expand All @@ -50,6 +60,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")
}
Expand All @@ -66,8 +77,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 }
Expand Down Expand Up @@ -97,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.
*/
Expand Down Expand Up @@ -130,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)
)
)
}

/**
Expand Down
31 changes: 21 additions & 10 deletions cpp/ql/lib/semmle/code/cpp/models/implementations/Scanf.qll
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import semmle.code.cpp.models.interfaces.Taint
import semmle.code.cpp.models.interfaces.Alias
import semmle.code.cpp.models.interfaces.SideEffect
import semmle.code.cpp.models.interfaces.FlowSource
private import semmle.code.cpp.security.FlowSources

/**
* The `scanf` family of functions.
Expand Down Expand Up @@ -72,21 +73,31 @@ abstract private class ScanfFunctionModel extends ArrayFunction, TaintFunction,
/**
* 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()
}
private class ScanfModel extends ScanfFunctionModel instanceof Scanf { }

abstract private class AbstractScanfFlowSource extends DataFlow::Node {
ScanfFunctionCall scanf;

AbstractScanfFlowSource() { this.asDefiningArgument(1) = scanf.getAnOutputArgument() }

string getSourceType() { result = "value read by " + scanf.getScanfFunction().getName() }
}

private class ScanfFlowSource extends AbstractScanfFlowSource, LocalFlowSource {
ScanfFlowSource() { not scanf.getScanfFunction() instanceof Fscanf }

final override string getSourceType() { result = AbstractScanfFlowSource.super.getSourceType() }
}

/**
* 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()
}
private class FscanfModel extends ScanfFunctionModel instanceof Fscanf { }

private class FScanfFlowSource extends AbstractScanfFlowSource, RemoteFlowSource {
FScanfFlowSource() { scanf.getScanfFunction() instanceof Fscanf }

final override string getSourceType() { result = AbstractScanfFlowSource.super.getSourceType() }
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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, // $ local_source
&n2); // $ local_source
}

{
int n;
fscanf_s(stream, "%d", &n); // $ remote_source
}

{
int n1, n2;
char buf[256];
scanf_s("%d %s",
&n1, // $ local_source
buf, // $ local_source
256,
&n2); // $ local_source
}

{
int n1, n2;
char buf[256];
fscanf_s(stream, "%d %s",
&n1, // $ remote_source
buf, // $ remote_source
256,
&n2); // $ remote_source
}
}
13 changes: 8 additions & 5 deletions cpp/ql/test/library-tests/scanf/scanfFormatLiteral.expected
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
| 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 |
| 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 |
9 changes: 5 additions & 4 deletions cpp/ql/test/library-tests/scanf/scanfFunctionCall.expected
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
| 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 |
| test.c:23:2:23:8 | call to scanf_s | 0 | 0 | test.c:23:10:23:19 | %d %s %d | non-wide |
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
| 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 |
| 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:38:23:40 | & ... | 2 |
5 changes: 5 additions & 0 deletions cpp/ql/test/library-tests/scanf/scanfFunctionCallOutput.ql
Original file line number Diff line number Diff line change
@@ -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
4 changes: 3 additions & 1 deletion cpp/ql/test/library-tests/scanf/test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Loading