Skip to content

Commit 6c765a9

Browse files
committed
Ruby: Fix bad join-order in BarrierGuard::getABarrierNode
Before ``` Evaluated relational algebra for predicate XSS#e59174e9::Shared::Sanitizer#class#f@6c9d334e with tuple counts: 0 ~0% {1} r1 = JOIN ActionView#3462bac2::RailsHtmlEscaping#f WITH project#DataFlowPublic#e1781e31::CallNode::getArgument#1#dispred#fff#3 ON FIRST 1 OUTPUT Lhs.0 554860 ~0% {2} r2 = JOIN SsaImpl#ff97b16a::Cached::getARead#1#ff_10#join_rhs WITH DataFlowPrivate#462ff392::Cached::TExprNode#ff ON FIRST 1 OUTPUT Lhs.1, Rhs.1 1 ~0% {1} r3 = JOIN r2 WITH DataFlowPublic#e1781e31::BarrierGuard#BarrierGuards#2462899b::stringConstArrayInclusionCall#::getAMaybeGuardedCapturedDef#0#f ON FIRST 1 OUTPUT Lhs.1 1 ~0% {1} r4 = r1 UNION r3 7 ~0% {1} r5 = JOIN r2 WITH DataFlowPublic#e1781e31::BarrierGuard#BarrierGuards#2462899b::stringConstCompare#::getAMaybeGuardedCapturedDef#0#f ON FIRST 1 OUTPUT Lhs.1 3045081 ~1% {3} r6 = JOIN DataFlowPrivate#462ff392::Cached::TExprNode#ff_10#join_rhs WITH DataFlowPrivate#462ff392::Cached::TExprNode#ff_10#join_rhs ON FIRST 1 OUTPUT Lhs.1, Lhs.0, Rhs.1 3045081 ~1% {3} r7 = JOIN r6 WITH ControlFlowGraph#46cebcbd::CfgNode::getBasicBlock#0#dispred#ff ON FIRST 1 OUTPUT Lhs.2, Lhs.1, Rhs.1 554860 ~1% {3} r8 = JOIN r7 WITH SsaImpl#ff97b16a::Cached::getARead#1#ff_10#join_rhs ON FIRST 1 OUTPUT Rhs.1, Lhs.1, Lhs.2 1462917146 ~0% {3} r9 = JOIN r8 WITH SsaImpl#ff97b16a::Cached::getARead#1#ff ON FIRST 1 OUTPUT Lhs.2, Lhs.1, Rhs.1 5082692 ~1% {4} r10 = JOIN r9 WITH DataFlowPublic#e1781e31::guardControlsBlock#3#fff_102#join_rhs ON FIRST 1 OUTPUT Rhs.1, Lhs.2, Rhs.2, Lhs.1 33 ~0% {1} r11 = JOIN r10 WITH BarrierGuards#2462899b::stringConstArrayInclusionCall#3#fff ON FIRST 3 OUTPUT Lhs.3 57 ~0% {1} r12 = JOIN r10 WITH BarrierGuards#2462899b::stringConstCompare#3#fff ON FIRST 3 OUTPUT Lhs.3 90 ~0% {1} r13 = r11 UNION r12 97 ~0% {1} r14 = r5 UNION r13 98 ~0% {1} r15 = r4 UNION r14 return r15 ``` After ``` [2022-10-17 20:35:01] Evaluated non-recursive predicate XSS#e59174e9::Shared::Sanitizer#class#f@487a64ar in 65ms (size: 98). Evaluated relational algebra for predicate XSS#e59174e9::Shared::Sanitizer#class#f@487a64ar with tuple counts: 0 ~0% {1} r1 = JOIN ActionView#3462bac2::RailsHtmlEscaping#f WITH project#DataFlowPublic#e1781e31::CallNode::getArgument#1#dispred#fff#3 ON FIRST 1 OUTPUT Lhs.0 33 ~0% {1} r2 = JOIN DataFlowPublic#e1781e31::BarrierGuard#BarrierGuards#2462899b::stringConstArrayInclusionCall#::guardChecksSsaDef#3#fff WITH DataFlowPublic#e1781e31::BarrierGuard#BarrierGuards#2462899b::stringConstArrayInclusionCall#::guardControlsSsaDef#4#ffff ON FIRST 3 OUTPUT Rhs.3 33 ~0% {1} r3 = r1 UNION r2 57 ~1% {1} r4 = JOIN DataFlowPublic#e1781e31::BarrierGuard#BarrierGuards#2462899b::stringConstCompare#::guardChecksSsaDef#3#fff WITH DataFlowPublic#e1781e31::BarrierGuard#BarrierGuards#2462899b::stringConstArrayInclusionCall#::guardControlsSsaDef#4#ffff ON FIRST 3 OUTPUT Rhs.3 554860 ~0% {2} r5 = JOIN SsaImpl#ff97b16a::Cached::getARead#1#ff_10#join_rhs WITH DataFlowPrivate#462ff392::Cached::TExprNode#ff ON FIRST 1 OUTPUT Lhs.1, Rhs.1 1 ~0% {1} r6 = JOIN r5 WITH DataFlowPublic#e1781e31::BarrierGuard#BarrierGuards#2462899b::stringConstArrayInclusionCall#::getAMaybeGuardedCapturedDef#0#f ON FIRST 1 OUTPUT Lhs.1 7 ~0% {1} r7 = JOIN r5 WITH DataFlowPublic#e1781e31::BarrierGuard#BarrierGuards#2462899b::stringConstCompare#::getAMaybeGuardedCapturedDef#0#f ON FIRST 1 OUTPUT Lhs.1 8 ~0% {1} r8 = r6 UNION r7 65 ~2% {1} r9 = r4 UNION r8 98 ~1% {1} r10 = r3 UNION r9 return r10 ```
1 parent c3968a2 commit 6c765a9

File tree

1 file changed

+16
-7
lines changed

1 file changed

+16
-7
lines changed

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -440,15 +440,24 @@ signature predicate guardChecksSig(CfgNodes::ExprCfgNode g, CfgNode e, boolean b
440440
* in data flow and taint tracking.
441441
*/
442442
module BarrierGuard<guardChecksSig/3 guardChecks> {
443+
pragma[nomagic]
444+
private predicate guardChecksSsaDef(CfgNodes::ExprCfgNode g, boolean branch, Ssa::Definition def) {
445+
guardChecks(g, def.getARead(), branch)
446+
}
447+
448+
pragma[nomagic]
449+
private predicate guardControlsSsaDef(
450+
CfgNodes::ExprCfgNode g, boolean branch, Ssa::Definition def, Node n
451+
) {
452+
def.getARead() = n.asExpr() and
453+
guardControlsBlock(g, n.asExpr().getBasicBlock(), branch)
454+
}
455+
443456
/** Gets a node that is safely guarded by the given guard check. */
444457
Node getABarrierNode() {
445-
exists(
446-
CfgNodes::ExprCfgNode g, boolean branch, CfgNodes::ExprCfgNode testedNode, Ssa::Definition def
447-
|
448-
def.getARead() = testedNode and
449-
def.getARead() = result.asExpr() and
450-
guardChecks(g, testedNode, branch) and
451-
guardControlsBlock(g, result.asExpr().getBasicBlock(), branch)
458+
exists(CfgNodes::ExprCfgNode g, boolean branch, Ssa::Definition def |
459+
guardChecksSsaDef(g, branch, def) and
460+
guardControlsSsaDef(g, branch, def, result)
452461
)
453462
or
454463
result.asExpr() = getAMaybeGuardedCapturedDef().getARead()

0 commit comments

Comments
 (0)