Skip to content

Make cpp/missing-check-scanf a path-problem query #15996

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 20, 2024
Merged

Make cpp/missing-check-scanf a path-problem query #15996

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
14aff5c
C++: Convert 'cpp/missing-check-scanf' to a path-problem query.
MathiasVP Mar 20, 2024
96cd259
C++: Add change note.
MathiasVP Mar 20, 2024
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
29 changes: 19 additions & 10 deletions cpp/ql/src/Critical/MissingCheckScanf.ql
View file
Open in desktop
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()
4 changes: 4 additions & 0 deletions cpp/ql/src/change-notes/2024-03-20-missing-check-scanf-path-problem.md
View file
Open in desktop
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