Skip to content

Commit

Permalink
Merge pull request #15996 from MathiasVP/missing-check-scanf-path-pro…
Browse files Browse the repository at this point in the history
…blem

Make `cpp/missing-check-scanf` a `path-problem` query
  • Loading branch information
MathiasVP authored Mar 20, 2024
2 parents 0fe3072 + 96cd259 commit e3be205
Show file tree
Hide file tree
Showing 3 changed files with 159 additions and 28 deletions.
29 changes: 19 additions & 10 deletions cpp/ql/src/Critical/MissingCheckScanf.ql
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* @name Missing return-value check for a 'scanf'-like function
* @description Failing to check that a call to 'scanf' actually writes to an
* output variable can lead to unexpected behavior at reading time.
* @kind problem
* @kind path-problem
* @problem.severity warning
* @security-severity 7.5
* @precision medium
Expand All @@ -20,6 +20,7 @@ import semmle.code.cpp.dataflow.new.DataFlow::DataFlow
import semmle.code.cpp.ir.IR
import semmle.code.cpp.ir.ValueNumbering
import ScanfChecks
import ScanfToUseFlow::PathGraph

/**
* Holds if `n` represents an uninitialized stack-allocated variable, or a
Expand Down Expand Up @@ -118,10 +119,13 @@ module ScanfToUseFlow = Global<ScanfToUseConfig>;
* Holds if `source` is the `index`'th argument to the `scanf`-like call `call`, and `sink` is
* a dataflow node that represents the expression `e`.
*/
predicate hasFlow(Node source, ScanfFunctionCall call, int index, Node sink, Expr e) {
isSource(call, index, source, _) and
ScanfToUseFlow::flow(source, sink) and
isSink(sink, e)
predicate flowPath(
ScanfToUseFlow::PathNode source, ScanfFunctionCall call, int index, ScanfToUseFlow::PathNode sink,
Expr e
) {
isSource(call, index, source.getNode(), _) and
ScanfToUseFlow::flowPath(source, sink) and
isSink(sink.getNode(), e)
}

/**
Expand All @@ -143,9 +147,12 @@ int getMinimumGuardConstant(ScanfFunctionCall call, int index) {
* Holds the access to `e` isn't guarded by a check that ensures that `call` returned
* at least `minGuard`.
*/
predicate hasNonGuardedAccess(ScanfFunctionCall call, Expr e, int minGuard) {
predicate hasNonGuardedAccess(
ScanfToUseFlow::PathNode source, ScanfFunctionCall call, ScanfToUseFlow::PathNode sink, Expr e,
int minGuard
) {
exists(int index |
hasFlow(_, call, index, _, e) and
flowPath(source, call, index, sink, e) and
minGuard = getMinimumGuardConstant(call, index)
|
not exists(int value |
Expand Down Expand Up @@ -173,9 +180,11 @@ BasicBlock blockGuardedBy(int value, string op, ScanfFunctionCall call) {
)
}

from ScanfFunctionCall call, Expr e, int minGuard
where hasNonGuardedAccess(call, e, minGuard)
select e,
from
ScanfToUseFlow::PathNode source, ScanfToUseFlow::PathNode sink, ScanfFunctionCall call, Expr e,
int minGuard
where hasNonGuardedAccess(source, call, sink, e, minGuard)
select e, source, sink,
"This variable is read, but may not have been written. " +
"It should be guarded by a check that the $@ returns at least " + minGuard + ".", call,
call.toString()
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The "Missing return-value check for a 'scanf'-like function" query (`cpp/missing-check-scanf`) has been converted to a `path-problem` query.
Loading

0 comments on commit e3be205

Please sign in to comment.