Skip to content

Commit 7b0492b

Browse files
committed
add flow tracking
1 parent bd0fd70 commit 7b0492b

File tree

1 file changed

+47
-10
lines changed

1 file changed

+47
-10
lines changed

cpp/src/security/UnsafeImplicitConversions/UnsafeImplicitConversions.ql

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ private import experimental.semmle.code.cpp.models.interfaces.SimpleRangeAnalysi
1616
private import experimental.semmle.code.cpp.rangeanalysis.RangeAnalysis
1717
private import semmle.code.cpp.ir.IR
1818
private import semmle.code.cpp.ir.ValueNumbering
19+
import semmle.code.cpp.dataflow.new.TaintTracking
20+
import semmle.code.cpp.models.interfaces.FlowSource
1921

2022
/**
2123
* Models standard I/O functions that return a length value bounded by their size argument
@@ -37,17 +39,18 @@ private class LenApproxFunc extends SimpleRangeAnalysisExpr, FunctionCall {
3739
* Uncomment the class below to silent findings that require large strings
3840
* to be passed to strlen to be exploitable.
3941
*/
40-
/*
41-
private class StrlenFunAssumption extends SimpleRangeAnalysisExpr, FunctionCall {
42-
StrlenFunAssumption() { this.getTarget().hasName("strlen") }
43-
44-
override float getLowerBounds() { result = 0 }
45-
46-
override float getUpperBounds() { result = 536870911 }
4742

48-
override predicate dependsOnChild(Expr child) { none() }
49-
}
50-
*/
43+
/*
44+
* private class StrlenFunAssumption extends SimpleRangeAnalysisExpr, FunctionCall {
45+
* StrlenFunAssumption() { this.getTarget().hasName("strlen") }
46+
*
47+
* override float getLowerBounds() { result = 0 }
48+
*
49+
* override float getUpperBounds() { result = 536870911 }
50+
*
51+
* override predicate dependsOnChild(Expr child) { none() }
52+
* }
53+
*/
5154

5255
/**
5356
* Determines if a function's address is taken in the codebase.
@@ -217,6 +220,33 @@ predicate safeBounds(Expr cast, IntegralType toType) {
217220
safeLowerBound(cast, toType) and safeUpperBound(cast, toType)
218221
}
219222

223+
/**
224+
* Taint tracking from user-controlled inputs to implicit conversions
225+
* UNUSED: uncomment the code below (near "select") to use
226+
*/
227+
module UnsafeUserInputConversionConfig implements DataFlow::ConfigSig {
228+
predicate isSource(DataFlow::Node source) {
229+
exists(RemoteFlowSourceFunction remoteFlow |
230+
remoteFlow = source.asExpr().(Call).getTarget() and
231+
remoteFlow.hasRemoteFlowSource(_, _)
232+
)
233+
or
234+
exists(LocalFlowSourceFunction localFlow |
235+
localFlow = source.asExpr().(Call).getTarget() and
236+
localFlow.hasLocalFlowSource(_, _)
237+
)
238+
}
239+
240+
predicate isSink(DataFlow::Node sink) {
241+
exists(IntegralConversion cast |
242+
cast.isImplicit() and
243+
cast.getExpr() = sink.asExpr()
244+
)
245+
}
246+
}
247+
248+
module UnsafeUserInputConversionFlow = TaintTracking::Global<UnsafeUserInputConversionConfig>;
249+
220250
from
221251
IntegralConversion cast, IntegralType fromType, IntegralType toType, Expr castExpr,
222252
string problemType
@@ -293,4 +323,11 @@ where
293323
or
294324
addressIsTaken(cast.getEnclosingFunction())
295325
)
326+
// Uncomment to report conversions with untrusted inputs only
327+
/*
328+
and exists(DataFlow::Node source, DataFlow::Node sink |
329+
cast.getExpr() = sink.asExpr() and
330+
UnsafeUserInputConversionFlow::flow(source, sink)
331+
)
332+
*/
296333
select cast, "Implicit cast from " + fromType + " to " + toType + " (" + problemType + ")"

0 commit comments

Comments
 (0)