Skip to content

Commit d7652f9

Browse files
committed
C++: Use a 'TaintTracking::Configuration' for 'cpp/overflow-destination'.
1 parent 275902d commit d7652f9

File tree

1 file changed

+47
-4
lines changed

1 file changed

+47
-4
lines changed

cpp/ql/src/Critical/OverflowDestination.ql

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@
1414
*/
1515

1616
import cpp
17-
import semmle.code.cpp.security.TaintTracking
17+
import semmle.code.cpp.ir.dataflow.TaintTracking
18+
import semmle.code.cpp.controlflow.IRGuards
19+
import semmle.code.cpp.security.FlowSources
20+
import DataFlow::PathGraph
1821

1922
/**
2023
* Holds if `fc` is a call to a copy operation where the size argument contains
@@ -45,9 +48,49 @@ predicate sourceSized(FunctionCall fc, Expr src) {
4548
)
4649
}
4750

48-
from FunctionCall fc, Expr vuln, Expr taintSource
51+
predicate readsVariable(LoadInstruction load, Variable var) {
52+
load.getSourceAddress().(VariableAddressInstruction).getASTVariable() = var
53+
}
54+
55+
predicate hasUpperBoundsCheck(Variable var) {
56+
exists(RelationalOperation oper, VariableAccess access |
57+
oper.getAnOperand() = access and
58+
access.getTarget() = var and
59+
// Comparing to 0 is not an upper bound check
60+
not oper.getAnOperand().getValue() = "0"
61+
)
62+
}
63+
64+
predicate nodeIsBarrierEqualityCandidate(DataFlow::Node node, Operand access, Variable checkedVar) {
65+
readsVariable(node.asInstruction(), checkedVar) and
66+
any(IRGuardCondition guard).ensuresEq(access, _, _, node.asInstruction().getBlock(), true)
67+
}
68+
69+
class OverflowDestinationConfig extends TaintTracking::Configuration {
70+
OverflowDestinationConfig() { this = "OverflowDestinationConfig" }
71+
72+
override predicate isSource(DataFlow::Node source) { source instanceof FlowSource }
73+
74+
override predicate isSink(DataFlow::Node sink) { sourceSized(_, sink.asExpr()) }
75+
76+
override predicate isSanitizer(DataFlow::Node node) {
77+
exists(Variable checkedVar |
78+
readsVariable(node.asInstruction(), checkedVar) and
79+
hasUpperBoundsCheck(checkedVar)
80+
)
81+
or
82+
exists(Variable checkedVar, Operand access |
83+
readsVariable(access.getDef(), checkedVar) and
84+
nodeIsBarrierEqualityCandidate(node, access, checkedVar)
85+
)
86+
}
87+
}
88+
89+
from
90+
FunctionCall fc, OverflowDestinationConfig conf, DataFlow::PathNode source,
91+
DataFlow::PathNode sink
4992
where
50-
sourceSized(fc, vuln) and
51-
tainted(taintSource, vuln)
93+
conf.hasFlowPath(source, sink) and
94+
sourceSized(fc, sink.getNode().asExpr())
5295
select fc,
5396
"To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size."

0 commit comments

Comments
 (0)