From 72356d1515283bce8f4d9bc9b19ef7a7d9aa044c Mon Sep 17 00:00:00 2001 From: Harry Maclean Date: Mon, 31 Jul 2023 17:17:54 +0100 Subject: [PATCH 1/6] Ruby: track flow from *args to positional params This models flow in the following case: def foo(x, y) sink x # 1 sink y # 2 end args = [source 1, source 2] foo(*args) We do this by introducing a SynthSplatParameterNode which accepts content from the splat argument, if one is given at the callsite. From this node we add read steps to each positional parameter. --- .../dataflow/internal/DataFlowDispatch.qll | 22 +++ .../dataflow/internal/DataFlowPrivate.qll | 91 ++++++++++++- .../dataflow/local/TaintStep.expected | 12 ++ .../dataflow/params/params-flow.expected | 126 +++++++++++++++--- .../dataflow/params/params_flow.rb | 38 +++++- .../type-tracker/TypeTracker.expected | 6 + 6 files changed, 270 insertions(+), 25 deletions(-) diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll index 2521311c814..195649beaff 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll @@ -439,6 +439,7 @@ private module Cached { } or THashSplatArgumentPosition() or TSplatAllArgumentPosition() or + TSplatArgumentPosition(int pos) { exists(Call c | c.getArgument(pos) instanceof SplatExpr) } or TAnyArgumentPosition() or TAnyKeywordArgumentPosition() @@ -468,6 +469,10 @@ private module Cached { // synthetic parameter position. TSynthHashSplatParameterPosition() or TSplatAllParameterPosition() or + TSplatParameterPosition(int pos) { + exists(Parameter p | p.getPosition() = pos and p instanceof SplatParameter) + } or + TSynthSplatParameterPosition() or TAnyParameterPosition() or TAnyKeywordParameterPosition() } @@ -1288,8 +1293,12 @@ class ParameterPosition extends TParameterPosition { predicate isSynthHashSplat() { this = TSynthHashSplatParameterPosition() } + predicate isSynthSplat() { this = TSynthSplatParameterPosition() } + predicate isSplatAll() { this = TSplatAllParameterPosition() } + predicate isSplat(int n) { this = TSplatParameterPosition(n) } + /** * Holds if this position represents any parameter, except `self` parameters. This * includes both positional, named, and block parameters. @@ -1320,6 +1329,10 @@ class ParameterPosition extends TParameterPosition { this.isAny() and result = "any" or this.isAnyNamed() and result = "any-named" + or + this.isSynthSplat() and result = "synthetic *" + or + exists(int pos | this.isSplat(pos) and result = "* (position " + pos + ")") } } @@ -1354,6 +1367,8 @@ class ArgumentPosition extends TArgumentPosition { predicate isSplatAll() { this = TSplatAllArgumentPosition() } + predicate isSplat(int n) { this = TSplatArgumentPosition(n) } + /** Gets a textual representation of this position. */ string toString() { this.isSelf() and result = "self" @@ -1371,6 +1386,8 @@ class ArgumentPosition extends TArgumentPosition { this.isHashSplat() and result = "**" or this.isSplatAll() and result = "*" + or + exists(int pos | this.isSplat(pos) and result = "* (position " + pos + ")") } } @@ -1408,6 +1425,11 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) { ppos.isAnyNamed() and apos.isKeyword(_) or apos.isAnyNamed() and ppos.isKeyword(_) + or + ppos.isSynthSplat() and apos.isSplatAll() + or + // Exact splat match + exists(int n | apos.isSplat(n) and ppos.isSplat(n)) } /** diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll index 70599e9a593..dd0f77dddf7 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll @@ -245,6 +245,7 @@ private class Argument extends CfgNodes::ExprCfgNode { not this.getExpr() instanceof BlockArgument and not this.getExpr().(Pair).getKey().getConstantValue().isSymbol(_) and not this.getExpr() instanceof HashSplatExpr and + not this.getExpr() instanceof SplatExpr and arg.isPositional(i) ) or @@ -261,8 +262,15 @@ private class Argument extends CfgNodes::ExprCfgNode { arg.isHashSplat() or this = call.getArgument(0) and + not exists(call.getArgument(1)) and this.getExpr() instanceof SplatExpr and arg.isSplatAll() + or + exists(int pos | pos > 0 or exists(call.getArgument(pos + 1)) | + this = call.getArgument(pos) and + this.getExpr() instanceof SplatExpr and + arg.isSplat(pos) + ) } /** Holds if this expression is the `i`th argument of `c`. */ @@ -300,6 +308,10 @@ private module Cached { TSynthHashSplatParameterNode(DataFlowCallable c) { isParameterNode(_, c, any(ParameterPosition p | p.isKeyword(_))) } or + TSynthSplatParameterNode(DataFlowCallable c) { + exists(c.asCallable()) and // exclude library callables + isParameterNode(_, c, any(ParameterPosition p | p.isPositional(_))) + } or TExprPostUpdateNode(CfgNodes::ExprCfgNode n) { // filter out nodes that clearly don't need post-update nodes isNonConstantExpr(n) and @@ -318,7 +330,7 @@ private module Cached { class TSourceParameterNode = TNormalParameterNode or TBlockParameterNode or TSelfParameterNode or - TSynthHashSplatParameterNode; + TSynthHashSplatParameterNode or TSynthSplatParameterNode; cached Location getLocation(NodeImpl n) { result = n.getLocationImpl() } @@ -514,6 +526,8 @@ predicate nodeIsHidden(Node n) { n instanceof SynthHashSplatParameterNode or n instanceof SynthHashSplatArgumentNode + or + n instanceof SynthSplatParameterNode } /** An SSA definition, viewed as a node in a data flow graph. */ @@ -610,7 +624,13 @@ private module ParameterNodes { pos.isHashSplat() or parameter = callable.getParameter(0).(SplatParameter) and + not exists(callable.getParameter(1)) and pos.isSplatAll() + or + exists(int n | n > 0 | + parameter = callable.getParameter(n).(SplatParameter) and + pos.isSplat(n) + ) ) } @@ -749,6 +769,66 @@ private module ParameterNodes { final override string toStringImpl() { result = "**kwargs" } } + /** + * A synthetic data-flow node to allow flow to positional parameters from a splat argument. + * + * For example, in the following code: + * + * ```rb + * def foo(x, y); end + * + * foo(*[a, b]) + * ``` + * + * We want `a` to flow to `x` and `b` to flow to `y`. We do this by constructing + * a `SynthSplatParameterNode` for the method `foo`, and matching the splat argument to this + * parameter node via `parameterMatch/2`. We then add read steps from this node to parameters + * `x` and `y`, for content at indices 0 and 1 respectively (see `readStep`). + * + * We don't yet correctly handle cases where the splat argument is not the first argument, e.g. in + * ```rb + * foo(a, *[b]) + * ``` + */ + class SynthSplatParameterNode extends ParameterNodeImpl, TSynthSplatParameterNode { + private DataFlowCallable callable; + + SynthSplatParameterNode() { this = TSynthSplatParameterNode(callable) } + + /** + * Gets a parameter which will contain the value given by `c`, assuming + * that the method was called with a single splat argument. + * For example, if the synth splat parameter is for the following method + * + * ```rb + * def foo(x, y, a:, *rest) + * end + * ``` + * + * Then `getAParameter(element 0) = x` and `getAParameter(element 1) = y`. + */ + ParameterNode getAParameter(ContentSet c) { + exists(int n | + isParameterNode(result, callable, (any(ParameterPosition p | p.isPositional(n)))) and + c = getPositionalContent(n) + ) + } + + final override Parameter getParameter() { none() } + + final override predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) { + c = callable and pos.isSynthSplat() + } + + final override CfgScope getCfgScope() { result = callable.asCallable() } + + final override DataFlowCallable getEnclosingCallable() { result = callable } + + final override Location getLocationImpl() { result = callable.getLocation() } + + final override string toStringImpl() { result = "synthetic *args" } + } + /** A parameter for a library callable with a flow summary. */ class SummaryParameterNode extends ParameterNodeImpl, FlowSummaryNode { private ParameterPosition pos_; @@ -1099,6 +1179,13 @@ private ContentSet getKeywordContent(string name) { ) } +private ContentSet getPositionalContent(int n) { + exists(ConstantValue::ConstantIntegerValue i | + result.isSingleton(TKnownElementContent(i)) and + i.isInt(n) + ) +} + /** * Subset of `storeStep` that should be shared with type-tracking. */ @@ -1187,6 +1274,8 @@ predicate readStep(Node node1, ContentSet c, Node node2) { or node2 = node1.(SynthHashSplatParameterNode).getAKeywordParameter(c) or + node2 = node1.(SynthSplatParameterNode).getAParameter(c) + or FlowSummaryImpl::Private::Steps::summaryReadStep(node1.(FlowSummaryNode).getSummaryNode(), c, node2.(FlowSummaryNode).getSummaryNode()) } diff --git a/ruby/ql/test/library-tests/dataflow/local/TaintStep.expected b/ruby/ql/test/library-tests/dataflow/local/TaintStep.expected index 525470654b6..1b4ac41e00b 100644 --- a/ruby/ql/test/library-tests/dataflow/local/TaintStep.expected +++ b/ruby/ql/test/library-tests/dataflow/local/TaintStep.expected @@ -2796,6 +2796,7 @@ | UseUseExplosion.rb:21:3675:21:3680 | call to use | UseUseExplosion.rb:21:3670:21:3680 | else ... | | UseUseExplosion.rb:21:3686:21:3696 | else ... | UseUseExplosion.rb:21:9:21:3700 | if ... | | UseUseExplosion.rb:21:3691:21:3696 | call to use | UseUseExplosion.rb:21:3686:21:3696 | else ... | +| UseUseExplosion.rb:24:5:25:7 | synthetic *args | UseUseExplosion.rb:24:13:24:13 | i | | UseUseExplosion.rb:24:5:25:7 | use | UseUseExplosion.rb:1:1:26:3 | C | | file://:0:0:0:0 | [summary param] position 0 in & | file://:0:0:0:0 | [summary] read: Argument[0].Element[any] in & | | file://:0:0:0:0 | [summary param] position 0 in + | file://:0:0:0:0 | [summary] read: Argument[0].Element[any] in + | @@ -2840,6 +2841,7 @@ | file://:0:0:0:0 | [summary] read: Argument[0].Element[any] in Hash[] | file://:0:0:0:0 | [summary] read: Argument[0].Element[any].Element[1] in Hash[] | | local_dataflow.rb:1:1:7:3 | self (foo) | local_dataflow.rb:3:8:3:10 | self | | local_dataflow.rb:1:1:7:3 | self in foo | local_dataflow.rb:1:1:7:3 | self (foo) | +| local_dataflow.rb:1:1:7:3 | synthetic *args | local_dataflow.rb:1:9:1:9 | a | | local_dataflow.rb:1:1:150:3 | self (local_dataflow.rb) | local_dataflow.rb:49:1:53:3 | self | | local_dataflow.rb:1:9:1:9 | a | local_dataflow.rb:1:9:1:9 | a | | local_dataflow.rb:1:9:1:9 | a | local_dataflow.rb:2:7:2:7 | a | @@ -2874,6 +2876,7 @@ | local_dataflow.rb:10:5:13:3 | __synth__0__1 | local_dataflow.rb:10:5:13:3 | __synth__0__1 | | local_dataflow.rb:10:5:13:3 | __synth__0__1 | local_dataflow.rb:10:9:10:9 | x | | local_dataflow.rb:10:5:13:3 | call to each | local_dataflow.rb:10:1:13:3 | ... = ... | +| local_dataflow.rb:10:5:13:3 | synthetic *args | local_dataflow.rb:10:5:13:3 | __synth__0__1 | | local_dataflow.rb:10:9:10:9 | x | local_dataflow.rb:12:5:12:5 | x | | local_dataflow.rb:10:14:10:18 | [post] array | local_dataflow.rb:15:10:15:14 | array | | local_dataflow.rb:10:14:10:18 | array | local_dataflow.rb:15:10:15:14 | array | @@ -2883,6 +2886,7 @@ | local_dataflow.rb:15:1:17:3 | __synth__0__1 | local_dataflow.rb:15:1:17:3 | __synth__0__1 | | local_dataflow.rb:15:1:17:3 | __synth__0__1 | local_dataflow.rb:15:1:17:3 | __synth__0__1 | | local_dataflow.rb:15:1:17:3 | __synth__0__1 | local_dataflow.rb:15:5:15:5 | x | +| local_dataflow.rb:15:1:17:3 | synthetic *args | local_dataflow.rb:15:1:17:3 | __synth__0__1 | | local_dataflow.rb:15:10:15:14 | [post] array | local_dataflow.rb:19:10:19:14 | array | | local_dataflow.rb:15:10:15:14 | array | local_dataflow.rb:19:10:19:14 | array | | local_dataflow.rb:16:9:16:10 | 10 | local_dataflow.rb:16:3:16:10 | break | @@ -2890,6 +2894,7 @@ | local_dataflow.rb:19:1:21:3 | __synth__0__1 | local_dataflow.rb:19:1:21:3 | __synth__0__1 | | local_dataflow.rb:19:1:21:3 | __synth__0__1 | local_dataflow.rb:19:1:21:3 | __synth__0__1 | | local_dataflow.rb:19:1:21:3 | __synth__0__1 | local_dataflow.rb:19:5:19:5 | x | +| local_dataflow.rb:19:1:21:3 | synthetic *args | local_dataflow.rb:19:1:21:3 | __synth__0__1 | | local_dataflow.rb:19:5:19:5 | x | local_dataflow.rb:20:6:20:6 | x | | local_dataflow.rb:20:6:20:6 | x | local_dataflow.rb:20:6:20:10 | ... > ... | | local_dataflow.rb:20:10:20:10 | 1 | local_dataflow.rb:20:6:20:10 | ... > ... | @@ -2901,11 +2906,13 @@ | local_dataflow.rb:30:14:30:20 | "class" | local_dataflow.rb:30:5:30:24 | C | | local_dataflow.rb:32:5:32:25 | bar | local_dataflow.rb:32:1:32:1 | x | | local_dataflow.rb:32:5:32:25 | bar | local_dataflow.rb:32:1:32:25 | ... = ... | +| local_dataflow.rb:34:1:39:3 | synthetic *args | local_dataflow.rb:34:7:34:7 | x | | local_dataflow.rb:34:7:34:7 | x | local_dataflow.rb:34:7:34:7 | x | | local_dataflow.rb:34:7:34:7 | x | local_dataflow.rb:35:6:35:6 | x | | local_dataflow.rb:35:6:35:6 | x | local_dataflow.rb:35:6:35:11 | ... == ... | | local_dataflow.rb:35:11:35:11 | 4 | local_dataflow.rb:35:6:35:11 | ... == ... | | local_dataflow.rb:36:13:36:13 | 7 | local_dataflow.rb:36:6:36:13 | return | +| local_dataflow.rb:41:1:47:3 | synthetic *args | local_dataflow.rb:41:7:41:7 | x | | local_dataflow.rb:41:7:41:7 | x | local_dataflow.rb:41:7:41:7 | x | | local_dataflow.rb:41:7:41:7 | x | local_dataflow.rb:42:6:42:6 | x | | local_dataflow.rb:42:6:42:6 | x | local_dataflow.rb:42:6:42:11 | ... == ... | @@ -2924,8 +2931,10 @@ | local_dataflow.rb:51:20:51:20 | x | local_dataflow.rb:51:20:51:24 | ... < ... | | local_dataflow.rb:51:24:51:24 | 9 | local_dataflow.rb:51:20:51:24 | ... < ... | | local_dataflow.rb:55:5:55:13 | Array | local_dataflow.rb:55:5:55:13 | call to [] | +| local_dataflow.rb:57:1:58:3 | synthetic *args | local_dataflow.rb:57:9:57:9 | x | | local_dataflow.rb:60:1:90:3 | self (test_case) | local_dataflow.rb:78:12:78:20 | self | | local_dataflow.rb:60:1:90:3 | self in test_case | local_dataflow.rb:60:1:90:3 | self (test_case) | +| local_dataflow.rb:60:1:90:3 | synthetic *args | local_dataflow.rb:60:15:60:15 | x | | local_dataflow.rb:60:15:60:15 | x | local_dataflow.rb:60:15:60:15 | x | | local_dataflow.rb:60:15:60:15 | x | local_dataflow.rb:61:12:61:12 | x | | local_dataflow.rb:61:7:68:5 | SSA phi read(x) | local_dataflow.rb:69:12:69:12 | x | @@ -3098,6 +3107,7 @@ | local_dataflow.rb:118:3:118:11 | call to source | local_dataflow.rb:118:3:118:31 | call to tap | | local_dataflow.rb:118:3:118:11 | self | local_dataflow.rb:119:3:119:31 | self | | local_dataflow.rb:118:17:118:31 | self | local_dataflow.rb:118:23:118:29 | self | +| local_dataflow.rb:118:17:118:31 | synthetic *args | local_dataflow.rb:118:20:118:20 | x | | local_dataflow.rb:118:20:118:20 | x | local_dataflow.rb:118:20:118:20 | x | | local_dataflow.rb:118:20:118:20 | x | local_dataflow.rb:118:28:118:28 | x | | local_dataflow.rb:119:3:119:31 | [post] self | local_dataflow.rb:119:8:119:16 | self | @@ -3112,8 +3122,10 @@ | local_dataflow.rb:123:8:123:20 | call to dup | local_dataflow.rb:123:8:123:45 | call to tap | | local_dataflow.rb:123:8:123:45 | call to tap | local_dataflow.rb:123:8:123:49 | call to dup | | local_dataflow.rb:123:26:123:45 | self | local_dataflow.rb:123:32:123:43 | self | +| local_dataflow.rb:123:26:123:45 | synthetic *args | local_dataflow.rb:123:29:123:29 | x | | local_dataflow.rb:126:1:128:3 | self (use) | local_dataflow.rb:127:3:127:8 | self | | local_dataflow.rb:126:1:128:3 | self in use | local_dataflow.rb:126:1:128:3 | self (use) | +| local_dataflow.rb:126:1:128:3 | synthetic *args | local_dataflow.rb:126:9:126:9 | x | | local_dataflow.rb:130:1:150:3 | self (use_use_madness) | local_dataflow.rb:132:6:132:11 | self | | local_dataflow.rb:130:1:150:3 | self in use_use_madness | local_dataflow.rb:130:1:150:3 | self (use_use_madness) | | local_dataflow.rb:131:3:131:3 | x | local_dataflow.rb:132:10:132:10 | x | diff --git a/ruby/ql/test/library-tests/dataflow/params/params-flow.expected b/ruby/ql/test/library-tests/dataflow/params/params-flow.expected index b54948d17a4..c1ebca59989 100644 --- a/ruby/ql/test/library-tests/dataflow/params/params-flow.expected +++ b/ruby/ql/test/library-tests/dataflow/params/params-flow.expected @@ -42,15 +42,49 @@ edges | params_flow.rb:41:24:41:29 | ** ... [element :p1] | params_flow.rb:16:13:16:14 | p1 | | params_flow.rb:41:26:41:29 | args [element :p1] | params_flow.rb:41:24:41:29 | ** ... [element :p1] | | params_flow.rb:44:12:44:20 | call to taint | params_flow.rb:9:16:9:17 | p1 | +| params_flow.rb:46:1:46:4 | args [element 0] | params_flow.rb:47:13:47:16 | args [element 0] | +| params_flow.rb:46:1:46:4 | args [element 1] | params_flow.rb:47:13:47:16 | args [element 1] | +| params_flow.rb:46:9:46:17 | call to taint | params_flow.rb:46:1:46:4 | args [element 0] | +| params_flow.rb:46:20:46:28 | call to taint | params_flow.rb:46:1:46:4 | args [element 1] | +| params_flow.rb:47:12:47:16 | * ... [element 0] | params_flow.rb:9:16:9:17 | p1 | +| params_flow.rb:47:12:47:16 | * ... [element 1] | params_flow.rb:9:20:9:21 | p2 | +| params_flow.rb:47:13:47:16 | args [element 0] | params_flow.rb:47:12:47:16 | * ... [element 0] | +| params_flow.rb:47:13:47:16 | args [element 1] | params_flow.rb:47:12:47:16 | * ... [element 1] | | params_flow.rb:49:13:49:14 | p1 | params_flow.rb:50:10:50:11 | p1 | -| params_flow.rb:54:9:54:17 | call to taint | params_flow.rb:49:13:49:14 | p1 | -| params_flow.rb:57:9:57:17 | call to taint | params_flow.rb:49:13:49:14 | p1 | -| params_flow.rb:62:1:62:4 | args | params_flow.rb:66:13:66:16 | args | -| params_flow.rb:62:8:62:16 | call to taint | params_flow.rb:62:1:62:4 | args | -| params_flow.rb:63:16:63:17 | *x [element 0] | params_flow.rb:64:10:64:10 | x [element 0] | -| params_flow.rb:64:10:64:10 | x [element 0] | params_flow.rb:64:10:64:13 | ...[...] | -| params_flow.rb:66:12:66:16 | * ... [element 0] | params_flow.rb:63:16:63:17 | *x [element 0] | -| params_flow.rb:66:13:66:16 | args | params_flow.rb:66:12:66:16 | * ... [element 0] | +| params_flow.rb:49:17:49:24 | *posargs [element 0] | params_flow.rb:51:11:51:17 | posargs [element 0] | +| params_flow.rb:51:11:51:17 | posargs [element 0] | params_flow.rb:51:11:51:20 | ...[...] | +| params_flow.rb:51:11:51:20 | ...[...] | params_flow.rb:51:10:51:21 | ( ... ) | +| params_flow.rb:55:9:55:17 | call to taint | params_flow.rb:49:13:49:14 | p1 | +| params_flow.rb:57:1:57:4 | args [element 0] | params_flow.rb:58:21:58:24 | args [element 0] | +| params_flow.rb:57:9:57:17 | call to taint | params_flow.rb:57:1:57:4 | args [element 0] | +| params_flow.rb:58:9:58:17 | call to taint | params_flow.rb:49:13:49:14 | p1 | +| params_flow.rb:58:20:58:24 | * ... [element 0] | params_flow.rb:49:17:49:24 | *posargs [element 0] | +| params_flow.rb:58:21:58:24 | args [element 0] | params_flow.rb:58:20:58:24 | * ... [element 0] | +| params_flow.rb:60:1:60:4 | args [element 0] | params_flow.rb:61:10:61:13 | args [element 0] | +| params_flow.rb:60:9:60:17 | call to taint | params_flow.rb:60:1:60:4 | args [element 0] | +| params_flow.rb:61:9:61:13 | * ... [element 0] | params_flow.rb:49:13:49:14 | p1 | +| params_flow.rb:61:10:61:13 | args [element 0] | params_flow.rb:61:9:61:13 | * ... [element 0] | +| params_flow.rb:63:1:63:4 | args | params_flow.rb:67:13:67:16 | args | +| params_flow.rb:63:8:63:16 | call to taint | params_flow.rb:63:1:63:4 | args | +| params_flow.rb:64:16:64:17 | *x [element 0] | params_flow.rb:65:10:65:10 | x [element 0] | +| params_flow.rb:65:10:65:10 | x [element 0] | params_flow.rb:65:10:65:13 | ...[...] | +| params_flow.rb:67:12:67:16 | * ... [element 0] | params_flow.rb:64:16:64:17 | *x [element 0] | +| params_flow.rb:67:13:67:16 | args | params_flow.rb:67:12:67:16 | * ... [element 0] | +| params_flow.rb:69:14:69:14 | x | params_flow.rb:70:10:70:10 | x | +| params_flow.rb:69:17:69:17 | y | params_flow.rb:71:10:71:10 | y | +| params_flow.rb:69:24:69:24 | w | params_flow.rb:74:10:74:10 | w | +| params_flow.rb:69:27:69:27 | r | params_flow.rb:75:10:75:10 | r | +| params_flow.rb:78:10:78:18 | call to taint | params_flow.rb:69:14:69:14 | x | +| params_flow.rb:78:21:78:29 | call to taint | params_flow.rb:69:17:69:17 | y | +| params_flow.rb:78:43:78:51 | call to taint | params_flow.rb:69:24:69:24 | w | +| params_flow.rb:78:54:78:62 | call to taint | params_flow.rb:69:27:69:27 | r | +| params_flow.rb:81:10:81:18 | call to taint | params_flow.rb:69:14:69:14 | x | +| params_flow.rb:83:14:83:14 | t | params_flow.rb:84:10:84:10 | t | +| params_flow.rb:83:17:83:17 | u | params_flow.rb:85:10:85:10 | u | +| params_flow.rb:83:23:83:23 | w | params_flow.rb:87:10:87:10 | w | +| params_flow.rb:94:10:94:18 | call to taint | params_flow.rb:83:14:83:14 | t | +| params_flow.rb:94:21:94:29 | call to taint | params_flow.rb:83:17:83:17 | u | +| params_flow.rb:94:39:94:47 | call to taint | params_flow.rb:83:23:83:23 | w | nodes | params_flow.rb:9:16:9:17 | p1 | semmle.label | p1 | | params_flow.rb:9:20:9:21 | p2 | semmle.label | p2 | @@ -100,22 +134,66 @@ nodes | params_flow.rb:41:24:41:29 | ** ... [element :p1] | semmle.label | ** ... [element :p1] | | params_flow.rb:41:26:41:29 | args [element :p1] | semmle.label | args [element :p1] | | params_flow.rb:44:12:44:20 | call to taint | semmle.label | call to taint | +| params_flow.rb:46:1:46:4 | args [element 0] | semmle.label | args [element 0] | +| params_flow.rb:46:1:46:4 | args [element 1] | semmle.label | args [element 1] | +| params_flow.rb:46:9:46:17 | call to taint | semmle.label | call to taint | +| params_flow.rb:46:20:46:28 | call to taint | semmle.label | call to taint | +| params_flow.rb:47:12:47:16 | * ... [element 0] | semmle.label | * ... [element 0] | +| params_flow.rb:47:12:47:16 | * ... [element 1] | semmle.label | * ... [element 1] | +| params_flow.rb:47:13:47:16 | args [element 0] | semmle.label | args [element 0] | +| params_flow.rb:47:13:47:16 | args [element 1] | semmle.label | args [element 1] | | params_flow.rb:49:13:49:14 | p1 | semmle.label | p1 | +| params_flow.rb:49:17:49:24 | *posargs [element 0] | semmle.label | *posargs [element 0] | | params_flow.rb:50:10:50:11 | p1 | semmle.label | p1 | -| params_flow.rb:54:9:54:17 | call to taint | semmle.label | call to taint | +| params_flow.rb:51:10:51:21 | ( ... ) | semmle.label | ( ... ) | +| params_flow.rb:51:11:51:17 | posargs [element 0] | semmle.label | posargs [element 0] | +| params_flow.rb:51:11:51:20 | ...[...] | semmle.label | ...[...] | +| params_flow.rb:55:9:55:17 | call to taint | semmle.label | call to taint | +| params_flow.rb:57:1:57:4 | args [element 0] | semmle.label | args [element 0] | | params_flow.rb:57:9:57:17 | call to taint | semmle.label | call to taint | -| params_flow.rb:62:1:62:4 | args | semmle.label | args | -| params_flow.rb:62:8:62:16 | call to taint | semmle.label | call to taint | -| params_flow.rb:63:16:63:17 | *x [element 0] | semmle.label | *x [element 0] | -| params_flow.rb:64:10:64:10 | x [element 0] | semmle.label | x [element 0] | -| params_flow.rb:64:10:64:13 | ...[...] | semmle.label | ...[...] | -| params_flow.rb:66:12:66:16 | * ... [element 0] | semmle.label | * ... [element 0] | -| params_flow.rb:66:13:66:16 | args | semmle.label | args | +| params_flow.rb:58:9:58:17 | call to taint | semmle.label | call to taint | +| params_flow.rb:58:20:58:24 | * ... [element 0] | semmle.label | * ... [element 0] | +| params_flow.rb:58:21:58:24 | args [element 0] | semmle.label | args [element 0] | +| params_flow.rb:60:1:60:4 | args [element 0] | semmle.label | args [element 0] | +| params_flow.rb:60:9:60:17 | call to taint | semmle.label | call to taint | +| params_flow.rb:61:9:61:13 | * ... [element 0] | semmle.label | * ... [element 0] | +| params_flow.rb:61:10:61:13 | args [element 0] | semmle.label | args [element 0] | +| params_flow.rb:63:1:63:4 | args | semmle.label | args | +| params_flow.rb:63:8:63:16 | call to taint | semmle.label | call to taint | +| params_flow.rb:64:16:64:17 | *x [element 0] | semmle.label | *x [element 0] | +| params_flow.rb:65:10:65:10 | x [element 0] | semmle.label | x [element 0] | +| params_flow.rb:65:10:65:13 | ...[...] | semmle.label | ...[...] | +| params_flow.rb:67:12:67:16 | * ... [element 0] | semmle.label | * ... [element 0] | +| params_flow.rb:67:13:67:16 | args | semmle.label | args | +| params_flow.rb:69:14:69:14 | x | semmle.label | x | +| params_flow.rb:69:17:69:17 | y | semmle.label | y | +| params_flow.rb:69:24:69:24 | w | semmle.label | w | +| params_flow.rb:69:27:69:27 | r | semmle.label | r | +| params_flow.rb:70:10:70:10 | x | semmle.label | x | +| params_flow.rb:71:10:71:10 | y | semmle.label | y | +| params_flow.rb:74:10:74:10 | w | semmle.label | w | +| params_flow.rb:75:10:75:10 | r | semmle.label | r | +| params_flow.rb:78:10:78:18 | call to taint | semmle.label | call to taint | +| params_flow.rb:78:21:78:29 | call to taint | semmle.label | call to taint | +| params_flow.rb:78:43:78:51 | call to taint | semmle.label | call to taint | +| params_flow.rb:78:54:78:62 | call to taint | semmle.label | call to taint | +| params_flow.rb:81:10:81:18 | call to taint | semmle.label | call to taint | +| params_flow.rb:83:14:83:14 | t | semmle.label | t | +| params_flow.rb:83:17:83:17 | u | semmle.label | u | +| params_flow.rb:83:23:83:23 | w | semmle.label | w | +| params_flow.rb:84:10:84:10 | t | semmle.label | t | +| params_flow.rb:85:10:85:10 | u | semmle.label | u | +| params_flow.rb:87:10:87:10 | w | semmle.label | w | +| params_flow.rb:94:10:94:18 | call to taint | semmle.label | call to taint | +| params_flow.rb:94:21:94:29 | call to taint | semmle.label | call to taint | +| params_flow.rb:94:39:94:47 | call to taint | semmle.label | call to taint | subpaths #select | params_flow.rb:10:10:10:11 | p1 | params_flow.rb:14:12:14:19 | call to taint | params_flow.rb:10:10:10:11 | p1 | $@ | params_flow.rb:14:12:14:19 | call to taint | call to taint | | params_flow.rb:10:10:10:11 | p1 | params_flow.rb:44:12:44:20 | call to taint | params_flow.rb:10:10:10:11 | p1 | $@ | params_flow.rb:44:12:44:20 | call to taint | call to taint | +| params_flow.rb:10:10:10:11 | p1 | params_flow.rb:46:9:46:17 | call to taint | params_flow.rb:10:10:10:11 | p1 | $@ | params_flow.rb:46:9:46:17 | call to taint | call to taint | | params_flow.rb:11:10:11:11 | p2 | params_flow.rb:14:22:14:29 | call to taint | params_flow.rb:11:10:11:11 | p2 | $@ | params_flow.rb:14:22:14:29 | call to taint | call to taint | +| params_flow.rb:11:10:11:11 | p2 | params_flow.rb:46:20:46:28 | call to taint | params_flow.rb:11:10:11:11 | p2 | $@ | params_flow.rb:46:20:46:28 | call to taint | call to taint | | params_flow.rb:17:10:17:11 | p1 | params_flow.rb:21:13:21:20 | call to taint | params_flow.rb:17:10:17:11 | p1 | $@ | params_flow.rb:21:13:21:20 | call to taint | call to taint | | params_flow.rb:17:10:17:11 | p1 | params_flow.rb:22:27:22:34 | call to taint | params_flow.rb:17:10:17:11 | p1 | $@ | params_flow.rb:22:27:22:34 | call to taint | call to taint | | params_flow.rb:17:10:17:11 | p1 | params_flow.rb:23:33:23:40 | call to taint | params_flow.rb:17:10:17:11 | p1 | $@ | params_flow.rb:23:33:23:40 | call to taint | call to taint | @@ -131,6 +209,16 @@ subpaths | params_flow.rb:28:10:28:22 | ( ... ) | params_flow.rb:37:34:37:42 | call to taint | params_flow.rb:28:10:28:22 | ( ... ) | $@ | params_flow.rb:37:34:37:42 | call to taint | call to taint | | params_flow.rb:29:10:29:22 | ( ... ) | params_flow.rb:33:41:33:49 | call to taint | params_flow.rb:29:10:29:22 | ( ... ) | $@ | params_flow.rb:33:41:33:49 | call to taint | call to taint | | params_flow.rb:29:10:29:22 | ( ... ) | params_flow.rb:34:14:34:22 | call to taint | params_flow.rb:29:10:29:22 | ( ... ) | $@ | params_flow.rb:34:14:34:22 | call to taint | call to taint | -| params_flow.rb:50:10:50:11 | p1 | params_flow.rb:54:9:54:17 | call to taint | params_flow.rb:50:10:50:11 | p1 | $@ | params_flow.rb:54:9:54:17 | call to taint | call to taint | -| params_flow.rb:50:10:50:11 | p1 | params_flow.rb:57:9:57:17 | call to taint | params_flow.rb:50:10:50:11 | p1 | $@ | params_flow.rb:57:9:57:17 | call to taint | call to taint | -| params_flow.rb:64:10:64:13 | ...[...] | params_flow.rb:62:8:62:16 | call to taint | params_flow.rb:64:10:64:13 | ...[...] | $@ | params_flow.rb:62:8:62:16 | call to taint | call to taint | +| params_flow.rb:50:10:50:11 | p1 | params_flow.rb:55:9:55:17 | call to taint | params_flow.rb:50:10:50:11 | p1 | $@ | params_flow.rb:55:9:55:17 | call to taint | call to taint | +| params_flow.rb:50:10:50:11 | p1 | params_flow.rb:58:9:58:17 | call to taint | params_flow.rb:50:10:50:11 | p1 | $@ | params_flow.rb:58:9:58:17 | call to taint | call to taint | +| params_flow.rb:50:10:50:11 | p1 | params_flow.rb:60:9:60:17 | call to taint | params_flow.rb:50:10:50:11 | p1 | $@ | params_flow.rb:60:9:60:17 | call to taint | call to taint | +| params_flow.rb:51:10:51:21 | ( ... ) | params_flow.rb:57:9:57:17 | call to taint | params_flow.rb:51:10:51:21 | ( ... ) | $@ | params_flow.rb:57:9:57:17 | call to taint | call to taint | +| params_flow.rb:65:10:65:13 | ...[...] | params_flow.rb:63:8:63:16 | call to taint | params_flow.rb:65:10:65:13 | ...[...] | $@ | params_flow.rb:63:8:63:16 | call to taint | call to taint | +| params_flow.rb:70:10:70:10 | x | params_flow.rb:78:10:78:18 | call to taint | params_flow.rb:70:10:70:10 | x | $@ | params_flow.rb:78:10:78:18 | call to taint | call to taint | +| params_flow.rb:70:10:70:10 | x | params_flow.rb:81:10:81:18 | call to taint | params_flow.rb:70:10:70:10 | x | $@ | params_flow.rb:81:10:81:18 | call to taint | call to taint | +| params_flow.rb:71:10:71:10 | y | params_flow.rb:78:21:78:29 | call to taint | params_flow.rb:71:10:71:10 | y | $@ | params_flow.rb:78:21:78:29 | call to taint | call to taint | +| params_flow.rb:74:10:74:10 | w | params_flow.rb:78:43:78:51 | call to taint | params_flow.rb:74:10:74:10 | w | $@ | params_flow.rb:78:43:78:51 | call to taint | call to taint | +| params_flow.rb:75:10:75:10 | r | params_flow.rb:78:54:78:62 | call to taint | params_flow.rb:75:10:75:10 | r | $@ | params_flow.rb:78:54:78:62 | call to taint | call to taint | +| params_flow.rb:84:10:84:10 | t | params_flow.rb:94:10:94:18 | call to taint | params_flow.rb:84:10:84:10 | t | $@ | params_flow.rb:94:10:94:18 | call to taint | call to taint | +| params_flow.rb:85:10:85:10 | u | params_flow.rb:94:21:94:29 | call to taint | params_flow.rb:85:10:85:10 | u | $@ | params_flow.rb:94:21:94:29 | call to taint | call to taint | +| params_flow.rb:87:10:87:10 | w | params_flow.rb:94:39:94:47 | call to taint | params_flow.rb:87:10:87:10 | w | $@ | params_flow.rb:94:39:94:47 | call to taint | call to taint | diff --git a/ruby/ql/test/library-tests/dataflow/params/params_flow.rb b/ruby/ql/test/library-tests/dataflow/params/params_flow.rb index 41ae58a58da..f2c038843bb 100644 --- a/ruby/ql/test/library-tests/dataflow/params/params_flow.rb +++ b/ruby/ql/test/library-tests/dataflow/params/params_flow.rb @@ -7,8 +7,8 @@ def sink x end def positional(p1, p2) - sink p1 # $ hasValueFlow=1 $ hasValueFlow=16 $ MISSING: hasValueFlow=18 - sink p2 # $ hasValueFlow=2 $ MISSING: hasValueFlow=17 $ MISSING: hasValueFlow=19 + sink p1 # $ hasValueFlow=1 $ hasValueFlow=16 $ hasValueFlow=18 + sink p2 # $ hasValueFlow=2 $ hasValueFlow=19 $ MISSING: hasValueFlow=17 end positional(taint(1), taint(2)) @@ -47,8 +47,9 @@ args = [taint(18), taint(19)] positional(*args) def posargs(p1, *posargs) - sink p1 # $ hasValueFlow=20 $ hasValueFlow=23 $ MISSING: hasValueFlow=24 - sink (posargs[0]) # $ MISSING: hasValueFlow=21 $ MISSING: hasValueFlow=22 $ MISSING: hasValueFlow=25 + sink p1 # $ hasValueFlow=20 $ hasValueFlow=23 $ hasValueFlow=24 + sink (posargs[0]) # $ hasValueFlow=22 $ MISSING: hasValueFlow=21 $ MISSING: hasValueFlow=25 + sink (posargs[1]) end posargs(taint(20), taint(21)) @@ -63,4 +64,31 @@ args = taint(26) def splatstuff(*x) sink x[0] # $ hasValueFlow=26 end -splatstuff(*args) \ No newline at end of file +splatstuff(*args) + +def splatmid(x, y, *z, w, r) + sink x # $ hasValueFlow=27 $ hasValueFlow=32 + sink y # $ hasValueFlow=28 $ MISSING: hasValueFlow=33 + sink z[0] # $ MISSING: hasValueFlow=29 $ MISSING: hasValueFlow=34 + sink z[1] # $ MISSING: hasValueFlow=35 + sink w # $ hasValueFlow=30 $ MISSING: hasValueFlow=36 + sink r # $ hasValueFlow=31 $ MISSING: hasValueFlow=37 +end + +splatmid(taint(27), taint(28), taint(29), taint(30), taint(31)) + +args = [taint(33), taint(34), taint(35), taint(36)] +splatmid(taint(32), *args, taint(37)) + +def pos_many(t, u, v, w, x, y, z) + sink t # $ hasValueFlow=38 + sink u # $ hasValueFlow=39 + sink v # $ MISSING: hasValueFlow=40 + sink w # $ MISSING: hasValueFlow=41 $ SPURIOUS: hasValueFlow=44 + sink x # $ MISSING: hasValueFlow=42 + sink y # $ MISSING: hasValueFlow=43 + sink z # $ MISSING: hasValueFlow=44 +end + +args = [taint(40), taint(41), taint(42), taint(43)] +pos_many(taint(38), taint(39), *args, taint(44)) \ No newline at end of file diff --git a/ruby/ql/test/library-tests/dataflow/type-tracker/TypeTracker.expected b/ruby/ql/test/library-tests/dataflow/type-tracker/TypeTracker.expected index 9a4277f4c0e..63116a97076 100644 --- a/ruby/ql/test/library-tests/dataflow/type-tracker/TypeTracker.expected +++ b/ruby/ql/test/library-tests/dataflow/type-tracker/TypeTracker.expected @@ -7,6 +7,7 @@ track | type_tracker.rb:2:5:5:7 | field= | type tracker without call steps | type_tracker.rb:2:5:5:7 | field= | | type_tracker.rb:2:5:5:7 | self in field= | type tracker with call steps | type_tracker.rb:7:5:9:7 | self in field | | type_tracker.rb:2:5:5:7 | self in field= | type tracker without call steps | type_tracker.rb:2:5:5:7 | self in field= | +| type_tracker.rb:2:5:5:7 | synthetic *args | type tracker without call steps | type_tracker.rb:2:5:5:7 | synthetic *args | | type_tracker.rb:2:16:2:18 | val | type tracker without call steps | type_tracker.rb:2:16:2:18 | val | | type_tracker.rb:2:16:2:18 | val | type tracker without call steps | type_tracker.rb:2:16:2:18 | val | | type_tracker.rb:2:16:2:18 | val | type tracker without call steps | type_tracker.rb:3:14:3:23 | call to field | @@ -46,6 +47,7 @@ track | type_tracker.rb:18:1:21:3 | &block | type tracker without call steps | type_tracker.rb:18:1:21:3 | &block | | type_tracker.rb:18:1:21:3 | positional | type tracker without call steps | type_tracker.rb:18:1:21:3 | positional | | type_tracker.rb:18:1:21:3 | self in positional | type tracker without call steps | type_tracker.rb:18:1:21:3 | self in positional | +| type_tracker.rb:18:1:21:3 | synthetic *args | type tracker without call steps | type_tracker.rb:18:1:21:3 | synthetic *args | | type_tracker.rb:18:16:18:17 | p1 | type tracker without call steps | type_tracker.rb:18:16:18:17 | p1 | | type_tracker.rb:18:16:18:17 | p1 | type tracker without call steps | type_tracker.rb:18:16:18:17 | p1 | | type_tracker.rb:18:20:18:21 | p2 | type tracker without call steps | type_tracker.rb:18:20:18:21 | p2 | @@ -118,6 +120,7 @@ track | type_tracker.rb:32:26:32:26 | 8 | type tracker without call steps with content element :p1 | type_tracker.rb:32:1:32:27 | ** | | type_tracker.rb:34:1:53:3 | &block | type tracker without call steps | type_tracker.rb:34:1:53:3 | &block | | type_tracker.rb:34:1:53:3 | self in throughArray | type tracker without call steps | type_tracker.rb:34:1:53:3 | self in throughArray | +| type_tracker.rb:34:1:53:3 | synthetic *args | type tracker without call steps | type_tracker.rb:34:1:53:3 | synthetic *args | | type_tracker.rb:34:1:53:3 | throughArray | type tracker without call steps | type_tracker.rb:34:1:53:3 | throughArray | | type_tracker.rb:34:18:34:20 | obj | type tracker without call steps | type_tracker.rb:34:18:34:20 | obj | | type_tracker.rb:34:18:34:20 | obj | type tracker without call steps | type_tracker.rb:34:18:34:20 | obj | @@ -272,6 +275,7 @@ trackEnd | type_tracker.rb:2:5:5:7 | self in field= | type_tracker.rb:7:5:9:7 | self (field) | | type_tracker.rb:2:5:5:7 | self in field= | type_tracker.rb:7:5:9:7 | self in field | | type_tracker.rb:2:5:5:7 | self in field= | type_tracker.rb:8:9:8:14 | self | +| type_tracker.rb:2:5:5:7 | synthetic *args | type_tracker.rb:2:5:5:7 | synthetic *args | | type_tracker.rb:2:16:2:18 | val | type_tracker.rb:2:16:2:18 | val | | type_tracker.rb:2:16:2:18 | val | type_tracker.rb:2:16:2:18 | val | | type_tracker.rb:2:16:2:18 | val | type_tracker.rb:2:16:2:18 | val | @@ -340,6 +344,7 @@ trackEnd | type_tracker.rb:18:1:21:3 | self in positional | type_tracker.rb:18:1:21:3 | self in positional | | type_tracker.rb:18:1:21:3 | self in positional | type_tracker.rb:19:5:19:11 | self | | type_tracker.rb:18:1:21:3 | self in positional | type_tracker.rb:20:5:20:11 | self | +| type_tracker.rb:18:1:21:3 | synthetic *args | type_tracker.rb:18:1:21:3 | synthetic *args | | type_tracker.rb:18:16:18:17 | p1 | type_tracker.rb:18:16:18:17 | p1 | | type_tracker.rb:18:16:18:17 | p1 | type_tracker.rb:18:16:18:17 | p1 | | type_tracker.rb:18:16:18:17 | p1 | type_tracker.rb:18:16:18:17 | p1 | @@ -427,6 +432,7 @@ trackEnd | type_tracker.rb:32:26:32:26 | 8 | type_tracker.rb:32:26:32:26 | 8 | | type_tracker.rb:34:1:53:3 | &block | type_tracker.rb:34:1:53:3 | &block | | type_tracker.rb:34:1:53:3 | self in throughArray | type_tracker.rb:34:1:53:3 | self in throughArray | +| type_tracker.rb:34:1:53:3 | synthetic *args | type_tracker.rb:34:1:53:3 | synthetic *args | | type_tracker.rb:34:1:53:3 | throughArray | type_tracker.rb:34:1:53:3 | throughArray | | type_tracker.rb:34:18:34:20 | obj | type_tracker.rb:34:18:34:20 | obj | | type_tracker.rb:34:18:34:20 | obj | type_tracker.rb:34:18:34:20 | obj | From c0baa5116fb296829f35b7904a2d1be83b5b23b5 Mon Sep 17 00:00:00 2001 From: Harry Maclean Date: Tue, 8 Aug 2023 09:13:42 +0100 Subject: [PATCH 2/6] Ruby: add test for example splat arg/param matches --- .../dataflow/params/params-flow.expected | 32 +++++++++++++++++++ .../dataflow/params/params_flow.rb | 16 ++++++---- 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/ruby/ql/test/library-tests/dataflow/params/params-flow.expected b/ruby/ql/test/library-tests/dataflow/params/params-flow.expected index c1ebca59989..2f384c3a877 100644 --- a/ruby/ql/test/library-tests/dataflow/params/params-flow.expected +++ b/ruby/ql/test/library-tests/dataflow/params/params-flow.expected @@ -72,8 +72,12 @@ edges | params_flow.rb:67:13:67:16 | args | params_flow.rb:67:12:67:16 | * ... [element 0] | | params_flow.rb:69:14:69:14 | x | params_flow.rb:70:10:70:10 | x | | params_flow.rb:69:17:69:17 | y | params_flow.rb:71:10:71:10 | y | +| params_flow.rb:69:20:69:21 | *z [element 0] | params_flow.rb:72:10:72:10 | z [element 0] | +| params_flow.rb:69:20:69:21 | *z [element 1] | params_flow.rb:73:10:73:10 | z [element 1] | | params_flow.rb:69:24:69:24 | w | params_flow.rb:74:10:74:10 | w | | params_flow.rb:69:27:69:27 | r | params_flow.rb:75:10:75:10 | r | +| params_flow.rb:72:10:72:10 | z [element 0] | params_flow.rb:72:10:72:13 | ...[...] | +| params_flow.rb:73:10:73:10 | z [element 1] | params_flow.rb:73:10:73:13 | ...[...] | | params_flow.rb:78:10:78:18 | call to taint | params_flow.rb:69:14:69:14 | x | | params_flow.rb:78:21:78:29 | call to taint | params_flow.rb:69:17:69:17 | y | | params_flow.rb:78:43:78:51 | call to taint | params_flow.rb:69:24:69:24 | w | @@ -85,6 +89,14 @@ edges | params_flow.rb:94:10:94:18 | call to taint | params_flow.rb:83:14:83:14 | t | | params_flow.rb:94:21:94:29 | call to taint | params_flow.rb:83:17:83:17 | u | | params_flow.rb:94:39:94:47 | call to taint | params_flow.rb:83:23:83:23 | w | +| params_flow.rb:96:10:96:18 | call to taint | params_flow.rb:69:14:69:14 | x | +| params_flow.rb:96:21:96:29 | call to taint | params_flow.rb:69:17:69:17 | y | +| params_flow.rb:96:32:96:65 | * ... [element 0] | params_flow.rb:69:20:69:21 | *z [element 0] | +| params_flow.rb:96:32:96:65 | * ... [element 1] | params_flow.rb:69:20:69:21 | *z [element 1] | +| params_flow.rb:96:34:96:42 | call to taint | params_flow.rb:96:32:96:65 | * ... [element 0] | +| params_flow.rb:96:45:96:53 | call to taint | params_flow.rb:96:32:96:65 | * ... [element 1] | +| params_flow.rb:96:68:96:76 | call to taint | params_flow.rb:69:24:69:24 | w | +| params_flow.rb:96:79:96:87 | call to taint | params_flow.rb:69:27:69:27 | r | nodes | params_flow.rb:9:16:9:17 | p1 | semmle.label | p1 | | params_flow.rb:9:20:9:21 | p2 | semmle.label | p2 | @@ -167,10 +179,16 @@ nodes | params_flow.rb:67:13:67:16 | args | semmle.label | args | | params_flow.rb:69:14:69:14 | x | semmle.label | x | | params_flow.rb:69:17:69:17 | y | semmle.label | y | +| params_flow.rb:69:20:69:21 | *z [element 0] | semmle.label | *z [element 0] | +| params_flow.rb:69:20:69:21 | *z [element 1] | semmle.label | *z [element 1] | | params_flow.rb:69:24:69:24 | w | semmle.label | w | | params_flow.rb:69:27:69:27 | r | semmle.label | r | | params_flow.rb:70:10:70:10 | x | semmle.label | x | | params_flow.rb:71:10:71:10 | y | semmle.label | y | +| params_flow.rb:72:10:72:10 | z [element 0] | semmle.label | z [element 0] | +| params_flow.rb:72:10:72:13 | ...[...] | semmle.label | ...[...] | +| params_flow.rb:73:10:73:10 | z [element 1] | semmle.label | z [element 1] | +| params_flow.rb:73:10:73:13 | ...[...] | semmle.label | ...[...] | | params_flow.rb:74:10:74:10 | w | semmle.label | w | | params_flow.rb:75:10:75:10 | r | semmle.label | r | | params_flow.rb:78:10:78:18 | call to taint | semmle.label | call to taint | @@ -187,6 +205,14 @@ nodes | params_flow.rb:94:10:94:18 | call to taint | semmle.label | call to taint | | params_flow.rb:94:21:94:29 | call to taint | semmle.label | call to taint | | params_flow.rb:94:39:94:47 | call to taint | semmle.label | call to taint | +| params_flow.rb:96:10:96:18 | call to taint | semmle.label | call to taint | +| params_flow.rb:96:21:96:29 | call to taint | semmle.label | call to taint | +| params_flow.rb:96:32:96:65 | * ... [element 0] | semmle.label | * ... [element 0] | +| params_flow.rb:96:32:96:65 | * ... [element 1] | semmle.label | * ... [element 1] | +| params_flow.rb:96:34:96:42 | call to taint | semmle.label | call to taint | +| params_flow.rb:96:45:96:53 | call to taint | semmle.label | call to taint | +| params_flow.rb:96:68:96:76 | call to taint | semmle.label | call to taint | +| params_flow.rb:96:79:96:87 | call to taint | semmle.label | call to taint | subpaths #select | params_flow.rb:10:10:10:11 | p1 | params_flow.rb:14:12:14:19 | call to taint | params_flow.rb:10:10:10:11 | p1 | $@ | params_flow.rb:14:12:14:19 | call to taint | call to taint | @@ -216,9 +242,15 @@ subpaths | params_flow.rb:65:10:65:13 | ...[...] | params_flow.rb:63:8:63:16 | call to taint | params_flow.rb:65:10:65:13 | ...[...] | $@ | params_flow.rb:63:8:63:16 | call to taint | call to taint | | params_flow.rb:70:10:70:10 | x | params_flow.rb:78:10:78:18 | call to taint | params_flow.rb:70:10:70:10 | x | $@ | params_flow.rb:78:10:78:18 | call to taint | call to taint | | params_flow.rb:70:10:70:10 | x | params_flow.rb:81:10:81:18 | call to taint | params_flow.rb:70:10:70:10 | x | $@ | params_flow.rb:81:10:81:18 | call to taint | call to taint | +| params_flow.rb:70:10:70:10 | x | params_flow.rb:96:10:96:18 | call to taint | params_flow.rb:70:10:70:10 | x | $@ | params_flow.rb:96:10:96:18 | call to taint | call to taint | | params_flow.rb:71:10:71:10 | y | params_flow.rb:78:21:78:29 | call to taint | params_flow.rb:71:10:71:10 | y | $@ | params_flow.rb:78:21:78:29 | call to taint | call to taint | +| params_flow.rb:71:10:71:10 | y | params_flow.rb:96:21:96:29 | call to taint | params_flow.rb:71:10:71:10 | y | $@ | params_flow.rb:96:21:96:29 | call to taint | call to taint | +| params_flow.rb:72:10:72:13 | ...[...] | params_flow.rb:96:34:96:42 | call to taint | params_flow.rb:72:10:72:13 | ...[...] | $@ | params_flow.rb:96:34:96:42 | call to taint | call to taint | +| params_flow.rb:73:10:73:13 | ...[...] | params_flow.rb:96:45:96:53 | call to taint | params_flow.rb:73:10:73:13 | ...[...] | $@ | params_flow.rb:96:45:96:53 | call to taint | call to taint | | params_flow.rb:74:10:74:10 | w | params_flow.rb:78:43:78:51 | call to taint | params_flow.rb:74:10:74:10 | w | $@ | params_flow.rb:78:43:78:51 | call to taint | call to taint | +| params_flow.rb:74:10:74:10 | w | params_flow.rb:96:68:96:76 | call to taint | params_flow.rb:74:10:74:10 | w | $@ | params_flow.rb:96:68:96:76 | call to taint | call to taint | | params_flow.rb:75:10:75:10 | r | params_flow.rb:78:54:78:62 | call to taint | params_flow.rb:75:10:75:10 | r | $@ | params_flow.rb:78:54:78:62 | call to taint | call to taint | +| params_flow.rb:75:10:75:10 | r | params_flow.rb:96:79:96:87 | call to taint | params_flow.rb:75:10:75:10 | r | $@ | params_flow.rb:96:79:96:87 | call to taint | call to taint | | params_flow.rb:84:10:84:10 | t | params_flow.rb:94:10:94:18 | call to taint | params_flow.rb:84:10:84:10 | t | $@ | params_flow.rb:94:10:94:18 | call to taint | call to taint | | params_flow.rb:85:10:85:10 | u | params_flow.rb:94:21:94:29 | call to taint | params_flow.rb:85:10:85:10 | u | $@ | params_flow.rb:94:21:94:29 | call to taint | call to taint | | params_flow.rb:87:10:87:10 | w | params_flow.rb:94:39:94:47 | call to taint | params_flow.rb:87:10:87:10 | w | $@ | params_flow.rb:94:39:94:47 | call to taint | call to taint | diff --git a/ruby/ql/test/library-tests/dataflow/params/params_flow.rb b/ruby/ql/test/library-tests/dataflow/params/params_flow.rb index f2c038843bb..0fa3cd1d743 100644 --- a/ruby/ql/test/library-tests/dataflow/params/params_flow.rb +++ b/ruby/ql/test/library-tests/dataflow/params/params_flow.rb @@ -67,12 +67,12 @@ end splatstuff(*args) def splatmid(x, y, *z, w, r) - sink x # $ hasValueFlow=27 $ hasValueFlow=32 - sink y # $ hasValueFlow=28 $ MISSING: hasValueFlow=33 - sink z[0] # $ MISSING: hasValueFlow=29 $ MISSING: hasValueFlow=34 - sink z[1] # $ MISSING: hasValueFlow=35 - sink w # $ hasValueFlow=30 $ MISSING: hasValueFlow=36 - sink r # $ hasValueFlow=31 $ MISSING: hasValueFlow=37 + sink x # $ hasValueFlow=27 $ hasValueFlow=32 $ hasValueFlow=45 + sink y # $ hasValueFlow=28 $ hasValueFlow=46 $ MISSING: hasValueFlow=33 + sink z[0] # $ hasValueFlow=47 $ MISSING: hasValueFlow=29 $ hasValueFlow=34 + sink z[1] # $ hasValueFlow=48 $ MISSING: hasValueFlow=35 + sink w # $ hasValueFlow=30 $ hasValueFlow=50 $ MISSING: hasValueFlow=36 + sink r # $ hasValueFlow=31 $ hasValueFlow=51 $ MISSING: hasValueFlow=37 end splatmid(taint(27), taint(28), taint(29), taint(30), taint(31)) @@ -91,4 +91,6 @@ def pos_many(t, u, v, w, x, y, z) end args = [taint(40), taint(41), taint(42), taint(43)] -pos_many(taint(38), taint(39), *args, taint(44)) \ No newline at end of file +pos_many(taint(38), taint(39), *args, taint(44)) + +splatmid(taint(45), taint(46), *[taint(47), taint(48), taint(49)], taint(50), taint(51)) From 6f3e2cdde3fc220155fe8433a949cf537ba390b1 Mon Sep 17 00:00:00 2001 From: Harry Maclean Date: Tue, 8 Aug 2023 16:37:35 +0100 Subject: [PATCH 3/6] Ruby: Add change note --- ruby/ql/lib/change-notes/2023-08-08-splat-arguments.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 ruby/ql/lib/change-notes/2023-08-08-splat-arguments.md diff --git a/ruby/ql/lib/change-notes/2023-08-08-splat-arguments.md b/ruby/ql/lib/change-notes/2023-08-08-splat-arguments.md new file mode 100644 index 00000000000..54786c50cfe --- /dev/null +++ b/ruby/ql/lib/change-notes/2023-08-08-splat-arguments.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Flow between splat arguments (`*args`) and positional parameters is now tracked more precisely. From 4239268efdc0a66417c8869e9962de3174bb53a1 Mon Sep 17 00:00:00 2001 From: Harry Maclean Date: Wed, 9 Aug 2023 14:43:27 +0100 Subject: [PATCH 4/6] Ruby: Prevent some false flow into splat params In cases where there are positional parameters after a splat parameter, don't attempt to match the splat parameter to a splat argument. We need more sophisticated modelling to handle these cases, which is future work. --- .../dataflow/internal/DataFlowPrivate.qll | 4 +- .../dataflow/params/params-flow.expected | 47 +++++++++++-------- .../dataflow/params/params_flow.rb | 22 ++++++++- 3 files changed, 50 insertions(+), 23 deletions(-) diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll index dd0f77dddf7..4fddc3d2584 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll @@ -629,7 +629,9 @@ private module ParameterNodes { or exists(int n | n > 0 | parameter = callable.getParameter(n).(SplatParameter) and - pos.isSplat(n) + pos.isSplat(n) and + // There are no positional parameters after the splat + not exists(SimpleParameter p, int m | m > n | p = callable.getParameter(m)) ) ) } diff --git a/ruby/ql/test/library-tests/dataflow/params/params-flow.expected b/ruby/ql/test/library-tests/dataflow/params/params-flow.expected index 2f384c3a877..fcfd1dcf533 100644 --- a/ruby/ql/test/library-tests/dataflow/params/params-flow.expected +++ b/ruby/ql/test/library-tests/dataflow/params/params-flow.expected @@ -72,12 +72,8 @@ edges | params_flow.rb:67:13:67:16 | args | params_flow.rb:67:12:67:16 | * ... [element 0] | | params_flow.rb:69:14:69:14 | x | params_flow.rb:70:10:70:10 | x | | params_flow.rb:69:17:69:17 | y | params_flow.rb:71:10:71:10 | y | -| params_flow.rb:69:20:69:21 | *z [element 0] | params_flow.rb:72:10:72:10 | z [element 0] | -| params_flow.rb:69:20:69:21 | *z [element 1] | params_flow.rb:73:10:73:10 | z [element 1] | | params_flow.rb:69:24:69:24 | w | params_flow.rb:74:10:74:10 | w | | params_flow.rb:69:27:69:27 | r | params_flow.rb:75:10:75:10 | r | -| params_flow.rb:72:10:72:10 | z [element 0] | params_flow.rb:72:10:72:13 | ...[...] | -| params_flow.rb:73:10:73:10 | z [element 1] | params_flow.rb:73:10:73:13 | ...[...] | | params_flow.rb:78:10:78:18 | call to taint | params_flow.rb:69:14:69:14 | x | | params_flow.rb:78:21:78:29 | call to taint | params_flow.rb:69:17:69:17 | y | | params_flow.rb:78:43:78:51 | call to taint | params_flow.rb:69:24:69:24 | w | @@ -91,12 +87,17 @@ edges | params_flow.rb:94:39:94:47 | call to taint | params_flow.rb:83:23:83:23 | w | | params_flow.rb:96:10:96:18 | call to taint | params_flow.rb:69:14:69:14 | x | | params_flow.rb:96:21:96:29 | call to taint | params_flow.rb:69:17:69:17 | y | -| params_flow.rb:96:32:96:65 | * ... [element 0] | params_flow.rb:69:20:69:21 | *z [element 0] | -| params_flow.rb:96:32:96:65 | * ... [element 1] | params_flow.rb:69:20:69:21 | *z [element 1] | -| params_flow.rb:96:34:96:42 | call to taint | params_flow.rb:96:32:96:65 | * ... [element 0] | -| params_flow.rb:96:45:96:53 | call to taint | params_flow.rb:96:32:96:65 | * ... [element 1] | | params_flow.rb:96:68:96:76 | call to taint | params_flow.rb:69:24:69:24 | w | | params_flow.rb:96:79:96:87 | call to taint | params_flow.rb:69:27:69:27 | r | +| params_flow.rb:98:19:98:19 | a | params_flow.rb:99:10:99:10 | a | +| params_flow.rb:98:31:98:31 | b | params_flow.rb:102:10:102:10 | b | +| params_flow.rb:105:15:105:23 | call to taint | params_flow.rb:98:19:98:19 | a | +| params_flow.rb:106:15:106:23 | call to taint | params_flow.rb:98:19:98:19 | a | +| params_flow.rb:106:37:106:45 | call to taint | params_flow.rb:98:31:98:31 | b | +| params_flow.rb:108:37:108:37 | a | params_flow.rb:109:10:109:10 | a | +| params_flow.rb:108:44:108:44 | c | params_flow.rb:111:10:111:10 | c | +| params_flow.rb:114:33:114:41 | call to taint | params_flow.rb:108:37:108:37 | a | +| params_flow.rb:114:58:114:66 | call to taint | params_flow.rb:108:44:108:44 | c | nodes | params_flow.rb:9:16:9:17 | p1 | semmle.label | p1 | | params_flow.rb:9:20:9:21 | p2 | semmle.label | p2 | @@ -179,16 +180,10 @@ nodes | params_flow.rb:67:13:67:16 | args | semmle.label | args | | params_flow.rb:69:14:69:14 | x | semmle.label | x | | params_flow.rb:69:17:69:17 | y | semmle.label | y | -| params_flow.rb:69:20:69:21 | *z [element 0] | semmle.label | *z [element 0] | -| params_flow.rb:69:20:69:21 | *z [element 1] | semmle.label | *z [element 1] | | params_flow.rb:69:24:69:24 | w | semmle.label | w | | params_flow.rb:69:27:69:27 | r | semmle.label | r | | params_flow.rb:70:10:70:10 | x | semmle.label | x | | params_flow.rb:71:10:71:10 | y | semmle.label | y | -| params_flow.rb:72:10:72:10 | z [element 0] | semmle.label | z [element 0] | -| params_flow.rb:72:10:72:13 | ...[...] | semmle.label | ...[...] | -| params_flow.rb:73:10:73:10 | z [element 1] | semmle.label | z [element 1] | -| params_flow.rb:73:10:73:13 | ...[...] | semmle.label | ...[...] | | params_flow.rb:74:10:74:10 | w | semmle.label | w | | params_flow.rb:75:10:75:10 | r | semmle.label | r | | params_flow.rb:78:10:78:18 | call to taint | semmle.label | call to taint | @@ -207,12 +202,21 @@ nodes | params_flow.rb:94:39:94:47 | call to taint | semmle.label | call to taint | | params_flow.rb:96:10:96:18 | call to taint | semmle.label | call to taint | | params_flow.rb:96:21:96:29 | call to taint | semmle.label | call to taint | -| params_flow.rb:96:32:96:65 | * ... [element 0] | semmle.label | * ... [element 0] | -| params_flow.rb:96:32:96:65 | * ... [element 1] | semmle.label | * ... [element 1] | -| params_flow.rb:96:34:96:42 | call to taint | semmle.label | call to taint | -| params_flow.rb:96:45:96:53 | call to taint | semmle.label | call to taint | | params_flow.rb:96:68:96:76 | call to taint | semmle.label | call to taint | | params_flow.rb:96:79:96:87 | call to taint | semmle.label | call to taint | +| params_flow.rb:98:19:98:19 | a | semmle.label | a | +| params_flow.rb:98:31:98:31 | b | semmle.label | b | +| params_flow.rb:99:10:99:10 | a | semmle.label | a | +| params_flow.rb:102:10:102:10 | b | semmle.label | b | +| params_flow.rb:105:15:105:23 | call to taint | semmle.label | call to taint | +| params_flow.rb:106:15:106:23 | call to taint | semmle.label | call to taint | +| params_flow.rb:106:37:106:45 | call to taint | semmle.label | call to taint | +| params_flow.rb:108:37:108:37 | a | semmle.label | a | +| params_flow.rb:108:44:108:44 | c | semmle.label | c | +| params_flow.rb:109:10:109:10 | a | semmle.label | a | +| params_flow.rb:111:10:111:10 | c | semmle.label | c | +| params_flow.rb:114:33:114:41 | call to taint | semmle.label | call to taint | +| params_flow.rb:114:58:114:66 | call to taint | semmle.label | call to taint | subpaths #select | params_flow.rb:10:10:10:11 | p1 | params_flow.rb:14:12:14:19 | call to taint | params_flow.rb:10:10:10:11 | p1 | $@ | params_flow.rb:14:12:14:19 | call to taint | call to taint | @@ -245,8 +249,6 @@ subpaths | params_flow.rb:70:10:70:10 | x | params_flow.rb:96:10:96:18 | call to taint | params_flow.rb:70:10:70:10 | x | $@ | params_flow.rb:96:10:96:18 | call to taint | call to taint | | params_flow.rb:71:10:71:10 | y | params_flow.rb:78:21:78:29 | call to taint | params_flow.rb:71:10:71:10 | y | $@ | params_flow.rb:78:21:78:29 | call to taint | call to taint | | params_flow.rb:71:10:71:10 | y | params_flow.rb:96:21:96:29 | call to taint | params_flow.rb:71:10:71:10 | y | $@ | params_flow.rb:96:21:96:29 | call to taint | call to taint | -| params_flow.rb:72:10:72:13 | ...[...] | params_flow.rb:96:34:96:42 | call to taint | params_flow.rb:72:10:72:13 | ...[...] | $@ | params_flow.rb:96:34:96:42 | call to taint | call to taint | -| params_flow.rb:73:10:73:13 | ...[...] | params_flow.rb:96:45:96:53 | call to taint | params_flow.rb:73:10:73:13 | ...[...] | $@ | params_flow.rb:96:45:96:53 | call to taint | call to taint | | params_flow.rb:74:10:74:10 | w | params_flow.rb:78:43:78:51 | call to taint | params_flow.rb:74:10:74:10 | w | $@ | params_flow.rb:78:43:78:51 | call to taint | call to taint | | params_flow.rb:74:10:74:10 | w | params_flow.rb:96:68:96:76 | call to taint | params_flow.rb:74:10:74:10 | w | $@ | params_flow.rb:96:68:96:76 | call to taint | call to taint | | params_flow.rb:75:10:75:10 | r | params_flow.rb:78:54:78:62 | call to taint | params_flow.rb:75:10:75:10 | r | $@ | params_flow.rb:78:54:78:62 | call to taint | call to taint | @@ -254,3 +256,8 @@ subpaths | params_flow.rb:84:10:84:10 | t | params_flow.rb:94:10:94:18 | call to taint | params_flow.rb:84:10:84:10 | t | $@ | params_flow.rb:94:10:94:18 | call to taint | call to taint | | params_flow.rb:85:10:85:10 | u | params_flow.rb:94:21:94:29 | call to taint | params_flow.rb:85:10:85:10 | u | $@ | params_flow.rb:94:21:94:29 | call to taint | call to taint | | params_flow.rb:87:10:87:10 | w | params_flow.rb:94:39:94:47 | call to taint | params_flow.rb:87:10:87:10 | w | $@ | params_flow.rb:94:39:94:47 | call to taint | call to taint | +| params_flow.rb:99:10:99:10 | a | params_flow.rb:105:15:105:23 | call to taint | params_flow.rb:99:10:99:10 | a | $@ | params_flow.rb:105:15:105:23 | call to taint | call to taint | +| params_flow.rb:99:10:99:10 | a | params_flow.rb:106:15:106:23 | call to taint | params_flow.rb:99:10:99:10 | a | $@ | params_flow.rb:106:15:106:23 | call to taint | call to taint | +| params_flow.rb:102:10:102:10 | b | params_flow.rb:106:37:106:45 | call to taint | params_flow.rb:102:10:102:10 | b | $@ | params_flow.rb:106:37:106:45 | call to taint | call to taint | +| params_flow.rb:109:10:109:10 | a | params_flow.rb:114:33:114:41 | call to taint | params_flow.rb:109:10:109:10 | a | $@ | params_flow.rb:114:33:114:41 | call to taint | call to taint | +| params_flow.rb:111:10:111:10 | c | params_flow.rb:114:58:114:66 | call to taint | params_flow.rb:111:10:111:10 | c | $@ | params_flow.rb:114:58:114:66 | call to taint | call to taint | diff --git a/ruby/ql/test/library-tests/dataflow/params/params_flow.rb b/ruby/ql/test/library-tests/dataflow/params/params_flow.rb index 0fa3cd1d743..371efb1a0de 100644 --- a/ruby/ql/test/library-tests/dataflow/params/params_flow.rb +++ b/ruby/ql/test/library-tests/dataflow/params/params_flow.rb @@ -69,8 +69,8 @@ splatstuff(*args) def splatmid(x, y, *z, w, r) sink x # $ hasValueFlow=27 $ hasValueFlow=32 $ hasValueFlow=45 sink y # $ hasValueFlow=28 $ hasValueFlow=46 $ MISSING: hasValueFlow=33 - sink z[0] # $ hasValueFlow=47 $ MISSING: hasValueFlow=29 $ hasValueFlow=34 - sink z[1] # $ hasValueFlow=48 $ MISSING: hasValueFlow=35 + sink z[0] # MISSING: $ hasValueFlow=47 $ hasValueFlow=29 $ hasValueFlow=34 + sink z[1] # $ MISSING: hasValueFlow=48 $ hasValueFlow=35 sink w # $ hasValueFlow=30 $ hasValueFlow=50 $ MISSING: hasValueFlow=36 sink r # $ hasValueFlow=31 $ hasValueFlow=51 $ MISSING: hasValueFlow=37 end @@ -94,3 +94,21 @@ args = [taint(40), taint(41), taint(42), taint(43)] pos_many(taint(38), taint(39), *args, taint(44)) splatmid(taint(45), taint(46), *[taint(47), taint(48), taint(49)], taint(50), taint(51)) + +def splatmidsmall(a, *splats, b) + sink a # $ hasValueFlow=52 $ hasValueFlow=55 + sink splats[0] # $ MISSING: hasValueFlow=53 + sink splats[1] # $ MISSING: hasValueFlow=54 + sink b # $ hasValueFlow=57 +end + +splatmidsmall(taint(52), *[taint(53), taint(54)]) +splatmidsmall(taint(55), taint(56), taint(57)) + +def splat_followed_by_keyword_param(a, *b, c:) + sink a # $ hasValueFlow=58 + sink b[0] # $ MISSING: hasValueFlow=59 + sink c # $ hasValueFlow=60 +end + +splat_followed_by_keyword_param(taint(58), taint(59), c: taint(60)) From 142393b5994d18dd5d505d977c864ca2880bb55f Mon Sep 17 00:00:00 2001 From: Harry Maclean Date: Wed, 9 Aug 2023 14:51:29 +0100 Subject: [PATCH 5/6] Ruby: Handle unknown content in splat flow --- .../codeql/ruby/dataflow/internal/DataFlowPrivate.qll | 6 +++++- .../dataflow/params/params-flow.expected | 11 +++++++++++ .../test/library-tests/dataflow/params/params_flow.rb | 8 ++++++-- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll index 4fddc3d2584..25b308fe95b 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll @@ -812,7 +812,11 @@ private module ParameterNodes { ParameterNode getAParameter(ContentSet c) { exists(int n | isParameterNode(result, callable, (any(ParameterPosition p | p.isPositional(n)))) and - c = getPositionalContent(n) + ( + c = getPositionalContent(n) + or + c.isSingleton(TUnknownElementContent()) + ) ) } diff --git a/ruby/ql/test/library-tests/dataflow/params/params-flow.expected b/ruby/ql/test/library-tests/dataflow/params/params-flow.expected index fcfd1dcf533..6ce6869c8d8 100644 --- a/ruby/ql/test/library-tests/dataflow/params/params-flow.expected +++ b/ruby/ql/test/library-tests/dataflow/params/params-flow.expected @@ -98,6 +98,11 @@ edges | params_flow.rb:108:44:108:44 | c | params_flow.rb:111:10:111:10 | c | | params_flow.rb:114:33:114:41 | call to taint | params_flow.rb:108:37:108:37 | a | | params_flow.rb:114:58:114:66 | call to taint | params_flow.rb:108:44:108:44 | c | +| params_flow.rb:117:1:117:1 | [post] x [element] | params_flow.rb:118:13:118:13 | x [element] | +| params_flow.rb:117:19:117:27 | call to taint | params_flow.rb:117:1:117:1 | [post] x [element] | +| params_flow.rb:118:12:118:13 | * ... [element] | params_flow.rb:9:16:9:17 | p1 | +| params_flow.rb:118:12:118:13 | * ... [element] | params_flow.rb:9:20:9:21 | p2 | +| params_flow.rb:118:13:118:13 | x [element] | params_flow.rb:118:12:118:13 | * ... [element] | nodes | params_flow.rb:9:16:9:17 | p1 | semmle.label | p1 | | params_flow.rb:9:20:9:21 | p2 | semmle.label | p2 | @@ -217,13 +222,19 @@ nodes | params_flow.rb:111:10:111:10 | c | semmle.label | c | | params_flow.rb:114:33:114:41 | call to taint | semmle.label | call to taint | | params_flow.rb:114:58:114:66 | call to taint | semmle.label | call to taint | +| params_flow.rb:117:1:117:1 | [post] x [element] | semmle.label | [post] x [element] | +| params_flow.rb:117:19:117:27 | call to taint | semmle.label | call to taint | +| params_flow.rb:118:12:118:13 | * ... [element] | semmle.label | * ... [element] | +| params_flow.rb:118:13:118:13 | x [element] | semmle.label | x [element] | subpaths #select | params_flow.rb:10:10:10:11 | p1 | params_flow.rb:14:12:14:19 | call to taint | params_flow.rb:10:10:10:11 | p1 | $@ | params_flow.rb:14:12:14:19 | call to taint | call to taint | | params_flow.rb:10:10:10:11 | p1 | params_flow.rb:44:12:44:20 | call to taint | params_flow.rb:10:10:10:11 | p1 | $@ | params_flow.rb:44:12:44:20 | call to taint | call to taint | | params_flow.rb:10:10:10:11 | p1 | params_flow.rb:46:9:46:17 | call to taint | params_flow.rb:10:10:10:11 | p1 | $@ | params_flow.rb:46:9:46:17 | call to taint | call to taint | +| params_flow.rb:10:10:10:11 | p1 | params_flow.rb:117:19:117:27 | call to taint | params_flow.rb:10:10:10:11 | p1 | $@ | params_flow.rb:117:19:117:27 | call to taint | call to taint | | params_flow.rb:11:10:11:11 | p2 | params_flow.rb:14:22:14:29 | call to taint | params_flow.rb:11:10:11:11 | p2 | $@ | params_flow.rb:14:22:14:29 | call to taint | call to taint | | params_flow.rb:11:10:11:11 | p2 | params_flow.rb:46:20:46:28 | call to taint | params_flow.rb:11:10:11:11 | p2 | $@ | params_flow.rb:46:20:46:28 | call to taint | call to taint | +| params_flow.rb:11:10:11:11 | p2 | params_flow.rb:117:19:117:27 | call to taint | params_flow.rb:11:10:11:11 | p2 | $@ | params_flow.rb:117:19:117:27 | call to taint | call to taint | | params_flow.rb:17:10:17:11 | p1 | params_flow.rb:21:13:21:20 | call to taint | params_flow.rb:17:10:17:11 | p1 | $@ | params_flow.rb:21:13:21:20 | call to taint | call to taint | | params_flow.rb:17:10:17:11 | p1 | params_flow.rb:22:27:22:34 | call to taint | params_flow.rb:17:10:17:11 | p1 | $@ | params_flow.rb:22:27:22:34 | call to taint | call to taint | | params_flow.rb:17:10:17:11 | p1 | params_flow.rb:23:33:23:40 | call to taint | params_flow.rb:17:10:17:11 | p1 | $@ | params_flow.rb:23:33:23:40 | call to taint | call to taint | diff --git a/ruby/ql/test/library-tests/dataflow/params/params_flow.rb b/ruby/ql/test/library-tests/dataflow/params/params_flow.rb index 371efb1a0de..3fcdfae9f64 100644 --- a/ruby/ql/test/library-tests/dataflow/params/params_flow.rb +++ b/ruby/ql/test/library-tests/dataflow/params/params_flow.rb @@ -7,8 +7,8 @@ def sink x end def positional(p1, p2) - sink p1 # $ hasValueFlow=1 $ hasValueFlow=16 $ hasValueFlow=18 - sink p2 # $ hasValueFlow=2 $ hasValueFlow=19 $ MISSING: hasValueFlow=17 + sink p1 # $ hasValueFlow=1 $ hasValueFlow=16 $ hasValueFlow=18 $ hasValueFlow=61 + sink p2 # $ hasValueFlow=2 $ hasValueFlow=19 $ hasValueFlow=61 $ MISSING: hasValueFlow=17 end positional(taint(1), taint(2)) @@ -112,3 +112,7 @@ def splat_followed_by_keyword_param(a, *b, c:) end splat_followed_by_keyword_param(taint(58), taint(59), c: taint(60)) + +x = [] +x[some_index()] = taint(61) +positional(*x) From b03f6efa60a0fb0d95a8552812ead78e12f3677e Mon Sep 17 00:00:00 2001 From: Harry Maclean Date: Wed, 9 Aug 2023 15:01:15 +0100 Subject: [PATCH 6/6] Ruby: Refactor --- .../codeql/ruby/dataflow/internal/DataFlowDispatch.qll | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll index 195649beaff..6052b462b27 100644 --- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll +++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll @@ -1418,6 +1418,11 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) { or ppos.isSplatAll() and apos.isSplatAll() or + ppos.isSynthSplat() and apos.isSplatAll() + or + // Exact splat match + exists(int n | apos.isSplat(n) and ppos.isSplat(n)) + or ppos.isAny() and argumentPositionIsNotSelf(apos) or apos.isAny() and parameterPositionIsNotSelf(ppos) @@ -1425,11 +1430,6 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) { ppos.isAnyNamed() and apos.isKeyword(_) or apos.isAnyNamed() and ppos.isKeyword(_) - or - ppos.isSynthSplat() and apos.isSplatAll() - or - // Exact splat match - exists(int n | apos.isSplat(n) and ppos.isSplat(n)) } /**