Skip to content
Open
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
5 changes: 5 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,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.
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())
}
Comment thread
MathiasVP marked this conversation as resolved.

/**
* 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`.
Comment thread
MathiasVP marked this conversation as resolved.
*/
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
22 changes: 16 additions & 6 deletions cpp/ql/lib/semmle/code/cpp/models/implementations/Scanf.qll
Original file line number Diff line number Diff line change
Expand Up @@ -69,23 +69,33 @@ 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)
}
}

/**
* 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) {
Expand Down
24 changes: 22 additions & 2 deletions cpp/ql/lib/semmle/code/cpp/models/interfaces/FlowSource.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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. */
Expand Down
5 changes: 2 additions & 3 deletions cpp/ql/lib/semmle/code/cpp/security/FlowSources.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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)
)
}
Expand All @@ -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)
)
}
Expand Down
5 changes: 1 addition & 4 deletions cpp/ql/src/Security/CWE/CWE-020/ExternalAPIsSpecific.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
3 changes: 1 addition & 2 deletions cpp/ql/src/Security/CWE/CWE-311/CleartextTransmission.ql
Original file line number Diff line number Diff line change
Expand Up @@ -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)
)
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