Merge pull request #13293 from MathiasVP/fix-performance-of-dtt

C++: Fix result duplication on `DefaultTaintTracking`
This commit is contained in:
Mathias Vorreiter Pedersen
2023-05-25 15:20:02 -07:00
committed by GitHub
2 changed files with 15 additions and 7 deletions

View File

@@ -1640,8 +1640,15 @@ predicate localInstructionFlow(Instruction e1, Instruction e2) {
localFlow(instructionNode(e1), instructionNode(e2))
}
/**
* INTERNAL: Do not use.
*
* Ideally this module would be private, but the `asExprInternal` predicate is
* needed in `DefaultTaintTrackingImpl`. Once `DefaultTaintTrackingImpl` is gone
* we can make this module private.
*/
cached
private module ExprFlowCached {
module ExprFlowCached {
/**
* Holds if `n` is an indirect operand of a `PointerArithmeticInstruction`, and
* `e` is the result of loading from the `PointerArithmeticInstruction`.
@@ -1692,7 +1699,8 @@ private module ExprFlowCached {
* `x[i]` steps to the expression `x[i - 1]` without traversing the
* entire chain.
*/
private Expr asExpr(Node n) {
cached
Expr asExprInternal(Node n) {
isIndirectBaseOfArrayAccess(n, result)
or
not isIndirectBaseOfArrayAccess(n, _) and
@@ -1704,7 +1712,7 @@ private module ExprFlowCached {
* dataflow step.
*/
private predicate localStepFromNonExpr(Node n1, Node n2) {
not exists(asExpr(n1)) and
not exists(asExprInternal(n1)) and
localFlowStep(n1, n2)
}
@@ -1715,7 +1723,7 @@ private module ExprFlowCached {
pragma[nomagic]
private predicate localStepsToExpr(Node n1, Node n2, Expr e2) {
localStepFromNonExpr*(n1, n2) and
e2 = asExpr(n2)
e2 = asExprInternal(n2)
}
/**
@@ -1726,7 +1734,7 @@ private module ExprFlowCached {
exists(Node mid |
localFlowStep(n1, mid) and
localStepsToExpr(mid, n2, e2) and
e1 = asExpr(n1)
e1 = asExprInternal(n1)
)
}

View File

@@ -60,7 +60,7 @@ private DataFlow::Node getNodeForSource(Expr source) {
}
private DataFlow::Node getNodeForExpr(Expr node) {
result = DataFlow::exprNode(node)
node = DataFlow::ExprFlowCached::asExprInternal(result)
or
// Some of the sources in `isUserInput` are intended to match the value of
// an expression, while others (those modeled below) are intended to match
@@ -221,7 +221,7 @@ private module Cached {
predicate nodeIsBarrierIn(DataFlow::Node node) {
// don't use dataflow into taint sources, as this leads to duplicate results.
exists(Expr source | isUserInput(source, _) |
node = DataFlow::exprNode(source)
source = DataFlow::ExprFlowCached::asExprInternal(node)
or
// This case goes together with the similar (but not identical) rule in
// `getNodeForSource`.