Ruby: Fix flow steps into phi nodes

- Add missing flow from post-update nodes into phi nodes.
- Prevent flow from reads into phi nodes when use-use flow is prohibited.
This commit is contained in:
Tom Hvitved 2022-10-21 15:16:19 +02:00
Родитель a191edfbd5
Коммит ee9163aa40
2 изменённых файлов: 58 добавлений и 30 удалений

Просмотреть файл

@ -76,19 +76,29 @@ module LocalFlow {
private import codeql.ruby.dataflow.internal.SsaImpl
/**
* Holds if `nodeFrom` is a last node referencing SSA definition `def`, which
* Holds if `nodeFrom` is a node for SSA definition `def`, which can reach `next`.
*/
private predicate localFlowSsaInputFromDef(
SsaDefinitionNode nodeFrom, Ssa::Definition def, Ssa::Definition next
) {
exists(BasicBlock bb, int i |
lastRefBeforeRedef(def, bb, i, next) and
def = nodeFrom.getDefinition() and
def.definesAt(_, bb, i)
)
}
/**
* Holds if `exprFrom` is a last read of SSA definition `def`, which
* can reach `next`.
*/
private predicate localFlowSsaInput(Node nodeFrom, Ssa::Definition def, Ssa::Definition next) {
exists(BasicBlock bb, int i | lastRefBeforeRedef(def, bb, i, next) |
def = nodeFrom.(SsaDefinitionNode).getDefinition() and
def.definesAt(_, bb, i)
or
exists(CfgNodes::ExprCfgNode e |
e = nodeFrom.asExpr() and
e = bb.getNode(i) and
e.getExpr() instanceof VariableReadAccess
)
predicate localFlowSsaInputFromExpr(
CfgNodes::ExprCfgNode exprFrom, Ssa::Definition def, Ssa::Definition next
) {
exists(BasicBlock bb, int i |
lastRefBeforeRedef(def, bb, i, next) and
exprFrom = bb.getNode(i) and
exprFrom.getExpr() instanceof VariableReadAccess
)
}
@ -139,9 +149,9 @@ module LocalFlow {
// Flow from read to next read
localSsaFlowStepUseUse(def, nodeFrom.(PostUpdateNode).getPreUpdateNode(), nodeTo)
or
// Flow into phi node
// Flow into phi node from definition
exists(Ssa::PhiNode phi |
localFlowSsaInput(nodeFrom, def, phi) and
localFlowSsaInputFromDef(nodeFrom, def, phi) and
phi = nodeTo.(SsaDefinitionNode).getDefinition() and
def = phi.getAnInput()
)
@ -308,6 +318,18 @@ private module Cached {
LocalFlow::localSsaFlowStepUseUse(_, nodeFrom, nodeTo) and
not FlowSummaryImpl::Private::Steps::prohibitsUseUseFlow(nodeFrom, _)
or
// Flow into phi node from read
exists(Ssa::Definition def, Ssa::PhiNode phi, CfgNodes::ExprCfgNode exprFrom |
LocalFlow::localFlowSsaInputFromExpr(exprFrom, def, phi) and
phi = nodeTo.(SsaDefinitionNode).getDefinition() and
def = phi.getAnInput()
|
exprFrom = nodeFrom.asExpr() and
not FlowSummaryImpl::Private::Steps::prohibitsUseUseFlow(nodeFrom, _)
or
exprFrom = nodeFrom.(PostUpdateNode).getPreUpdateNode().asExpr()
)
or
FlowSummaryImpl::Private::Steps::summaryLocalStep(nodeFrom, nodeTo, true)
}
@ -338,6 +360,14 @@ private module Cached {
)
or
LocalFlow::localSsaFlowStepUseUse(_, nodeFrom, nodeTo)
or
// Flow into phi node from read
exists(Ssa::Definition def, Ssa::PhiNode phi, CfgNodes::ExprCfgNode exprFrom |
LocalFlow::localFlowSsaInputFromExpr(exprFrom, def, phi) and
phi = nodeTo.(SsaDefinitionNode).getDefinition() and
def = phi.getAnInput() and
exprFrom = [nodeFrom.asExpr(), nodeFrom.(PostUpdateNode).getPreUpdateNode().asExpr()]
)
}
private predicate entrySsaDefinition(SsaDefinitionNode n) {

Просмотреть файл

@ -1,22 +1,20 @@
failures
| ssa_flow.rb:16:16:16:33 | # $ hasValueFlow=1 | Missing result:hasValueFlow=1 |
| ssa_flow.rb:29:10:29:13 | ...[...] | Unexpected result: hasValueFlow=2 |
edges
| ssa_flow.rb:24:9:24:9 | [post] a [element 0] : | ssa_flow.rb:29:10:29:10 | a [element 0] : |
| ssa_flow.rb:24:9:24:9 | [post] a [element 0] : | ssa_flow.rb:29:10:29:10 | a [element 0] : |
| ssa_flow.rb:24:16:24:23 | call to taint : | ssa_flow.rb:24:9:24:9 | [post] a [element 0] : |
| ssa_flow.rb:24:16:24:23 | call to taint : | ssa_flow.rb:24:9:24:9 | [post] a [element 0] : |
| ssa_flow.rb:29:10:29:10 | a [element 0] : | ssa_flow.rb:29:10:29:13 | ...[...] |
| ssa_flow.rb:29:10:29:10 | a [element 0] : | ssa_flow.rb:29:10:29:13 | ...[...] |
| ssa_flow.rb:12:9:12:9 | [post] a [element 0] : | ssa_flow.rb:16:10:16:10 | a [element 0] : |
| ssa_flow.rb:12:9:12:9 | [post] a [element 0] : | ssa_flow.rb:16:10:16:10 | a [element 0] : |
| ssa_flow.rb:12:16:12:23 | call to taint : | ssa_flow.rb:12:9:12:9 | [post] a [element 0] : |
| ssa_flow.rb:12:16:12:23 | call to taint : | ssa_flow.rb:12:9:12:9 | [post] a [element 0] : |
| ssa_flow.rb:16:10:16:10 | a [element 0] : | ssa_flow.rb:16:10:16:13 | ...[...] |
| ssa_flow.rb:16:10:16:10 | a [element 0] : | ssa_flow.rb:16:10:16:13 | ...[...] |
nodes
| ssa_flow.rb:24:9:24:9 | [post] a [element 0] : | semmle.label | [post] a [element 0] : |
| ssa_flow.rb:24:9:24:9 | [post] a [element 0] : | semmle.label | [post] a [element 0] : |
| ssa_flow.rb:24:16:24:23 | call to taint : | semmle.label | call to taint : |
| ssa_flow.rb:24:16:24:23 | call to taint : | semmle.label | call to taint : |
| ssa_flow.rb:29:10:29:10 | a [element 0] : | semmle.label | a [element 0] : |
| ssa_flow.rb:29:10:29:10 | a [element 0] : | semmle.label | a [element 0] : |
| ssa_flow.rb:29:10:29:13 | ...[...] | semmle.label | ...[...] |
| ssa_flow.rb:29:10:29:13 | ...[...] | semmle.label | ...[...] |
| ssa_flow.rb:12:9:12:9 | [post] a [element 0] : | semmle.label | [post] a [element 0] : |
| ssa_flow.rb:12:9:12:9 | [post] a [element 0] : | semmle.label | [post] a [element 0] : |
| ssa_flow.rb:12:16:12:23 | call to taint : | semmle.label | call to taint : |
| ssa_flow.rb:12:16:12:23 | call to taint : | semmle.label | call to taint : |
| ssa_flow.rb:16:10:16:10 | a [element 0] : | semmle.label | a [element 0] : |
| ssa_flow.rb:16:10:16:10 | a [element 0] : | semmle.label | a [element 0] : |
| ssa_flow.rb:16:10:16:13 | ...[...] | semmle.label | ...[...] |
| ssa_flow.rb:16:10:16:13 | ...[...] | semmle.label | ...[...] |
subpaths
#select
| ssa_flow.rb:29:10:29:13 | ...[...] | ssa_flow.rb:24:16:24:23 | call to taint : | ssa_flow.rb:29:10:29:13 | ...[...] | $@ | ssa_flow.rb:24:16:24:23 | call to taint : | call to taint : |
| ssa_flow.rb:16:10:16:13 | ...[...] | ssa_flow.rb:12:16:12:23 | call to taint : | ssa_flow.rb:16:10:16:13 | ...[...] | $@ | ssa_flow.rb:12:16:12:23 | call to taint : | call to taint : |