From b16b95e2f73a41e937a10a190e2f69e22c27aa14 Mon Sep 17 00:00:00 2001 From: Nick Rolfe Date: Wed, 5 May 2021 12:12:45 +0100 Subject: [PATCH] Fix type-tracking load/store steps --- ql/src/codeql_ruby/controlflow/CfgNodes.qll | 5 +++++ .../typetracking/TypeTrackerSpecific.qll | 15 ++++++--------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/ql/src/codeql_ruby/controlflow/CfgNodes.qll b/ql/src/codeql_ruby/controlflow/CfgNodes.qll index 10449ed7c91..2c6f9f7a809 100644 --- a/ql/src/codeql_ruby/controlflow/CfgNodes.qll +++ b/ql/src/codeql_ruby/controlflow/CfgNodes.qll @@ -247,6 +247,11 @@ module ExprNodes { override predicate relevantChild(Expr e) { e = this.getValue() or e = this.getBranch(_) } } + /** A control-flow node that wraps a `MethodCall` AST expression. */ + class MethodCallCfgNode extends CallCfgNode { + MethodCallCfgNode() { this.getExpr() instanceof MethodCall } + } + /** A control-flow node that wraps a `CaseExpr` AST expression. */ class CaseExprCfgNode extends ExprCfgNode { override CaseExprChildMapping e; diff --git a/ql/src/codeql_ruby/typetracking/TypeTrackerSpecific.qll b/ql/src/codeql_ruby/typetracking/TypeTrackerSpecific.qll index abef22a19ea..5c96fe28029 100644 --- a/ql/src/codeql_ruby/typetracking/TypeTrackerSpecific.qll +++ b/ql/src/codeql_ruby/typetracking/TypeTrackerSpecific.qll @@ -72,15 +72,12 @@ predicate returnStep(DataFlowPrivate::ReturnNode nodeFrom, Node nodeTo) { */ predicate basicStoreStep(Node nodeFrom, LocalSourceNode nodeTo, string content) { // TODO: support SetterMethodCall inside TuplePattern - exists( - ExprNodes::AssignmentCfgNode assignment, ExprNodes::CallCfgNode call, - DataFlowPublic::ExprNode receiver - | + exists(ExprNodes::AssignmentCfgNode assignment, ExprNodes::CallCfgNode call | assignment.getLhs() = call and content = getSetterCallAttributeName(call.getExpr()) and - receiver.getExprNode().getNode() = call.getExpr().(AST::SetterMethodCall).getReceiver() and - assignment.getRhs() = nodeFrom.(DataFlowPublic::ExprNode).getExprNode() and - nodeTo = receiver + nodeTo.(DataFlowPublic::ExprNode).getExprNode() = call.getReceiver() and + call.getExpr() instanceof AST::SetterMethodCall and + assignment.getRhs() = nodeFrom.(DataFlowPublic::ExprNode).getExprNode() ) } @@ -105,10 +102,10 @@ private string getSetterCallAttributeName(AST::SetterMethodCall call) { * Holds if `nodeTo` is the result of accessing the `content` content of `nodeFrom`. */ predicate basicLoadStep(Node nodeFrom, Node nodeTo, string content) { - exists(ExprNodes::CallCfgNode call | + exists(ExprNodes::MethodCallCfgNode call | call.getExpr().getNumberOfArguments() = 0 and content = call.getExpr().(AST::MethodCall).getMethodName() and - nodeFrom.asExpr().getNode() = call.getExpr().(AST::MethodCall).getReceiver() and + nodeFrom.asExpr() = call.getReceiver() and nodeTo.asExpr() = call ) }