Ruby: Flow through hash-splat parameters

This commit is contained in:
Tom Hvitved 2022-05-25 10:36:36 +02:00
Родитель 67572bb770
Коммит a7b39ebeca
7 изменённых файлов: 142 добавлений и 2 удалений

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

@ -9,5 +9,7 @@ private class MyConsistencyConfiguration extends ConsistencyConfiguration {
n instanceof BlockArgumentNode
or
n instanceof SummaryNode
or
n instanceof HashSplatArgumentsNode
}
}

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

@ -259,7 +259,8 @@ private module Cached {
exists(any(Call c).getKeywordArgument(name))
or
FlowSummaryImplSpecific::ParsePositions::isParsedKeywordParameterPosition(_, name)
}
} or
THashSplatArgumentPosition()
cached
newtype TParameterPosition =
@ -278,6 +279,7 @@ private module Cached {
or
FlowSummaryImplSpecific::ParsePositions::isParsedKeywordArgumentPosition(_, name)
} or
THashSplatParameterPosition() or
TAnyParameterPosition()
}
@ -476,6 +478,9 @@ class ParameterPosition extends TParameterPosition {
/** Holds if this position represents a keyword parameter named `name`. */
predicate isKeyword(string name) { this = TKeywordParameterPosition(name) }
/** Holds if this position represents a hash-splat parameter. */
predicate isHashSplat() { this = THashSplatParameterPosition() }
/**
* Holds if this position represents any parameter. This includes both positional
* and named parameters.
@ -494,6 +499,8 @@ class ParameterPosition extends TParameterPosition {
or
exists(string name | this.isKeyword(name) and result = "keyword " + name)
or
this.isHashSplat() and result = "**"
or
this.isAny() and result = "any"
}
}
@ -512,6 +519,12 @@ class ArgumentPosition extends TArgumentPosition {
/** Holds if this position represents a keyword argument named `name`. */
predicate isKeyword(string name) { this = TKeywordArgumentPosition(name) }
/**
* Holds if this position represents a synthesized argument containing all keyword
* arguments wrapped in a hash.
*/
predicate isHashSplat() { this = THashSplatArgumentPosition() }
/** Gets a textual representation of this position. */
string toString() {
this.isSelf() and result = "self"
@ -521,6 +534,8 @@ class ArgumentPosition extends TArgumentPosition {
exists(int pos | this.isPositional(pos) and result = "position " + pos)
or
exists(string name | this.isKeyword(name) and result = "keyword " + name)
or
this.isHashSplat() and result = "**"
}
}
@ -539,5 +554,7 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) {
or
exists(string name | ppos.isKeyword(name) and apos.isKeyword(name))
or
ppos.isHashSplat() and apos.isHashSplat()
or
ppos.isAny() and exists(apos)
}

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

@ -216,7 +216,8 @@ private module Cached {
TNormalParameterNode(Parameter p) {
p instanceof SimpleParameter or
p instanceof OptionalParameter or
p instanceof KeywordParameter
p instanceof KeywordParameter or
p instanceof HashSplatParameter
} or
TSelfParameterNode(MethodBase m) or
TBlockParameterNode(MethodBase m) or
@ -232,6 +233,9 @@ private module Cached {
} or
TSummaryParameterNode(FlowSummaryImpl::Public::SummarizedCallable c, ParameterPosition pos) {
FlowSummaryImpl::Private::summaryParameterNodeRange(c, pos)
} or
THashSplatArgumentsNode(CfgNodes::ExprNodes::CallCfgNode c) {
exists(Argument arg | arg.isArgumentOf(c, any(ArgumentPosition pos | pos.isKeyword(_))))
}
class TParameterNode =
@ -389,6 +393,8 @@ predicate nodeIsHidden(Node n) {
n instanceof SummaryParameterNode
or
n instanceof SynthReturnNode
or
n instanceof HashSplatArgumentsNode
}
/** An SSA definition, viewed as a node in a data flow graph. */
@ -473,6 +479,9 @@ private module ParameterNodes {
c.getAParameter() = kp and
pos.isKeyword(kp.getName())
)
or
parameter = c.getAParameter().(HashSplatParameter) and
pos.isHashSplat()
}
override CfgScope getCfgScope() { result = parameter.getCallable() }
@ -651,6 +660,40 @@ private module ArgumentNodes {
FlowSummaryImpl::Private::summaryArgumentNode(call, this, pos)
}
}
/**
* A data-flow node that represents all keyword arguments wrapped in a hash.
*
* The callee is responsible for filtering out the keyword arguments that are
* part of the method signature, such that those cannot end up in the hash-splat
* parameter.
*/
class HashSplatArgumentsNode extends ArgumentNode, THashSplatArgumentsNode {
CfgNodes::ExprNodes::CallCfgNode c;
HashSplatArgumentsNode() { this = THashSplatArgumentsNode(c) }
override predicate argumentOf(DataFlowCall call, ArgumentPosition pos) {
this.sourceArgumentOf(call.asCall(), pos)
}
override predicate sourceArgumentOf(CfgNodes::ExprNodes::CallCfgNode call, ArgumentPosition pos) {
call = c and
pos.isHashSplat()
}
}
private class HashSplatArgumentsNodeImpl extends NodeImpl, THashSplatArgumentsNode {
CfgNodes::ExprNodes::CallCfgNode c;
HashSplatArgumentsNodeImpl() { this = THashSplatArgumentsNode(c) }
override CfgScope getCfgScope() { result = c.getExpr().getCfgScope() }
override Location getLocationImpl() { result = c.getLocation() }
override string toStringImpl() { result = "**" }
}
}
import ArgumentNodes
@ -807,6 +850,13 @@ predicate jumpStep(Node pred, Node succ) {
succ.asExpr().getExpr().(ConstantReadAccess).getValue() = pred.asExpr().getExpr()
}
private ContentSet getKeywordContent(string name) {
exists(ConstantValue::ConstantSymbolValue key |
result.isSingleton(TKnownElementContent(key)) and
key.isSymbol(name)
)
}
/**
* Holds if data can flow from `node1` to `node2` via an assignment to
* content `c`.
@ -845,6 +895,14 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
c.isSingleton(TUnknownPairValueContent())
)
)
or
// Wrap all keyword arguments in a synthesized hash-splat argument node
exists(CfgNodes::ExprNodes::CallCfgNode call, ArgumentPosition keywordPos, string name |
node2 = THashSplatArgumentsNode(call) and
node1.asExpr().(Argument).isArgumentOf(call, keywordPos) and
keywordPos.isKeyword(name) and
c = getKeywordContent(name)
)
}
/**
@ -870,6 +928,19 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
*/
predicate clearsContent(Node n, ContentSet c) {
FlowSummaryImpl::Private::Steps::summaryClearsContent(n, c)
or
// Filter out keyword arguments that are part of the method signature from
// the hash-splat parameter
exists(
DataFlowCallable callable, ParameterPosition hashSplatPos, ParameterNodeImpl keywordParam,
ParameterPosition keywordPos, string name
|
n.(ParameterNodes::NormalParameterNode).isParameterOf(callable, hashSplatPos) and
hashSplatPos.isHashSplat() and
keywordParam.isParameterOf(callable, keywordPos) and
keywordPos.isKeyword(name) and
c = getKeywordContent(name)
)
}
/**

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

@ -12,6 +12,16 @@ edges
| params_flow.rb:22:27:22:34 | call to taint : | params_flow.rb:16:13:16:14 | p1 : |
| params_flow.rb:23:16:23:23 | call to taint : | params_flow.rb:16:18:16:19 | p2 : |
| params_flow.rb:23:33:23:40 | call to taint : | params_flow.rb:16:13:16:14 | p1 : |
| params_flow.rb:25:12:25:13 | p1 : | params_flow.rb:26:10:26:11 | p1 |
| params_flow.rb:25:17:25:24 | **kwargs [element :p2] : | params_flow.rb:28:11:28:16 | kwargs [element :p2] : |
| params_flow.rb:25:17:25:24 | **kwargs [element :p3] : | params_flow.rb:29:11:29:16 | kwargs [element :p3] : |
| params_flow.rb:28:11:28:16 | kwargs [element :p2] : | params_flow.rb:28:11:28:21 | ...[...] : |
| params_flow.rb:28:11:28:21 | ...[...] : | params_flow.rb:28:10:28:22 | ( ... ) |
| params_flow.rb:29:11:29:16 | kwargs [element :p3] : | params_flow.rb:29:11:29:21 | ...[...] : |
| params_flow.rb:29:11:29:21 | ...[...] : | params_flow.rb:29:10:29:22 | ( ... ) |
| params_flow.rb:33:12:33:19 | call to taint : | params_flow.rb:25:12:25:13 | p1 : |
| params_flow.rb:33:26:33:34 | call to taint : | params_flow.rb:25:17:25:24 | **kwargs [element :p2] : |
| params_flow.rb:33:41:33:49 | call to taint : | params_flow.rb:25:17:25:24 | **kwargs [element :p3] : |
nodes
| params_flow.rb:9:16:9:17 | p1 : | semmle.label | p1 : |
| params_flow.rb:9:20:9:21 | p2 : | semmle.label | p2 : |
@ -29,6 +39,19 @@ nodes
| params_flow.rb:22:27:22:34 | call to taint : | semmle.label | call to taint : |
| params_flow.rb:23:16:23:23 | call to taint : | semmle.label | call to taint : |
| params_flow.rb:23:33:23:40 | call to taint : | semmle.label | call to taint : |
| params_flow.rb:25:12:25:13 | p1 : | semmle.label | p1 : |
| params_flow.rb:25:17:25:24 | **kwargs [element :p2] : | semmle.label | **kwargs [element :p2] : |
| params_flow.rb:25:17:25:24 | **kwargs [element :p3] : | semmle.label | **kwargs [element :p3] : |
| params_flow.rb:26:10:26:11 | p1 | semmle.label | p1 |
| params_flow.rb:28:10:28:22 | ( ... ) | semmle.label | ( ... ) |
| params_flow.rb:28:11:28:16 | kwargs [element :p2] : | semmle.label | kwargs [element :p2] : |
| params_flow.rb:28:11:28:21 | ...[...] : | semmle.label | ...[...] : |
| params_flow.rb:29:10:29:22 | ( ... ) | semmle.label | ( ... ) |
| params_flow.rb:29:11:29:16 | kwargs [element :p3] : | semmle.label | kwargs [element :p3] : |
| params_flow.rb:29:11:29:21 | ...[...] : | semmle.label | ...[...] : |
| params_flow.rb:33:12:33:19 | call to taint : | semmle.label | call to taint : |
| params_flow.rb:33:26:33:34 | call to taint : | semmle.label | call to taint : |
| params_flow.rb:33:41:33:49 | 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 : |
@ -39,3 +62,6 @@ subpaths
| params_flow.rb:18:10:18:11 | p2 | params_flow.rb:21:27:21:34 | call to taint : | params_flow.rb:18:10:18:11 | p2 | $@ | params_flow.rb:21:27:21:34 | call to taint : | call to taint : |
| params_flow.rb:18:10:18:11 | p2 | params_flow.rb:22:13:22:20 | call to taint : | params_flow.rb:18:10:18:11 | p2 | $@ | params_flow.rb:22:13:22:20 | call to taint : | call to taint : |
| params_flow.rb:18:10:18:11 | p2 | params_flow.rb:23:16:23:23 | call to taint : | params_flow.rb:18:10:18:11 | p2 | $@ | params_flow.rb:23:16:23:23 | call to taint : | call to taint : |
| params_flow.rb:26:10:26:11 | p1 | params_flow.rb:33:12:33:19 | call to taint : | params_flow.rb:26:10:26:11 | p1 | $@ | params_flow.rb:33:12:33:19 | call to taint : | call to taint : |
| params_flow.rb:28:10:28:22 | ( ... ) | params_flow.rb:33:26:33:34 | call to taint : | params_flow.rb:28:10:28:22 | ( ... ) | $@ | params_flow.rb:33:26:33:34 | 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 : |

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

@ -21,3 +21,13 @@ end
keyword(p1: taint(3), p2: taint(4))
keyword(p2: taint(5), p1: taint(6))
keyword(:p2 => taint(7), :p1 => taint(8))
def kwargs(p1:, **kwargs)
sink p1 # $ hasValueFlow=9
sink (kwargs[:p1])
sink (kwargs[:p2]) # $ hasValueFlow=10
sink (kwargs[:p3]) # $ hasValueFlow=11
sink (kwargs[:p4])
end
kwargs(p1: taint(9), p2: taint(10), p3: taint(11), p4: "")

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

@ -139,6 +139,7 @@ track
| type_tracker.rb:27:5:27:11 | call to puts | type tracker without call steps | type_tracker.rb:31:1:31:21 | call to keyword |
| type_tracker.rb:27:5:27:11 | call to puts | type tracker without call steps | type_tracker.rb:32:1:32:27 | call to keyword |
| type_tracker.rb:27:10:27:11 | [post] p2 | type tracker without call steps | type_tracker.rb:27:10:27:11 | [post] p2 |
| type_tracker.rb:30:1:30:21 | ** | type tracker without call steps | type_tracker.rb:30:1:30:21 | ** |
| type_tracker.rb:30:1:30:21 | [post] self | type tracker with call steps | type_tracker.rb:25:1:28:3 | self (keyword) |
| type_tracker.rb:30:1:30:21 | [post] self | type tracker with call steps | type_tracker.rb:25:1:28:3 | self in keyword |
| type_tracker.rb:30:1:30:21 | [post] self | type tracker without call steps | type_tracker.rb:30:1:30:21 | [post] self |
@ -155,6 +156,7 @@ track
| type_tracker.rb:30:20:30:20 | 4 | type tracker with call steps | type_tracker.rb:25:18:25:19 | p2 |
| type_tracker.rb:30:20:30:20 | 4 | type tracker without call steps | type_tracker.rb:30:20:30:20 | 4 |
| type_tracker.rb:30:20:30:20 | [post] 4 | type tracker without call steps | type_tracker.rb:30:20:30:20 | [post] 4 |
| type_tracker.rb:31:1:31:21 | ** | type tracker without call steps | type_tracker.rb:31:1:31:21 | ** |
| type_tracker.rb:31:1:31:21 | [post] self | type tracker with call steps | type_tracker.rb:25:1:28:3 | self (keyword) |
| type_tracker.rb:31:1:31:21 | [post] self | type tracker with call steps | type_tracker.rb:25:1:28:3 | self in keyword |
| type_tracker.rb:31:1:31:21 | [post] self | type tracker without call steps | type_tracker.rb:31:1:31:21 | [post] self |
@ -171,6 +173,7 @@ track
| type_tracker.rb:31:20:31:20 | 6 | type tracker with call steps | type_tracker.rb:25:13:25:14 | p1 |
| type_tracker.rb:31:20:31:20 | 6 | type tracker without call steps | type_tracker.rb:31:20:31:20 | 6 |
| type_tracker.rb:31:20:31:20 | [post] 6 | type tracker without call steps | type_tracker.rb:31:20:31:20 | [post] 6 |
| type_tracker.rb:32:1:32:27 | ** | type tracker without call steps | type_tracker.rb:32:1:32:27 | ** |
| type_tracker.rb:32:1:32:27 | [post] self | type tracker without call steps | type_tracker.rb:32:1:32:27 | [post] self |
| type_tracker.rb:32:1:32:27 | call to keyword | type tracker without call steps | type_tracker.rb:32:1:32:27 | call to keyword |
| type_tracker.rb:32:9:32:11 | :p2 | type tracker without call steps | type_tracker.rb:32:9:32:11 | :p2 |
@ -393,6 +396,7 @@ trackEnd
| type_tracker.rb:27:5:27:11 | call to puts | type_tracker.rb:31:1:31:21 | call to keyword |
| type_tracker.rb:27:5:27:11 | call to puts | type_tracker.rb:32:1:32:27 | call to keyword |
| type_tracker.rb:27:10:27:11 | [post] p2 | type_tracker.rb:27:10:27:11 | [post] p2 |
| type_tracker.rb:30:1:30:21 | ** | type_tracker.rb:30:1:30:21 | ** |
| type_tracker.rb:30:1:30:21 | [post] self | type_tracker.rb:25:1:28:3 | self (keyword) |
| type_tracker.rb:30:1:30:21 | [post] self | type_tracker.rb:25:1:28:3 | self in keyword |
| type_tracker.rb:30:1:30:21 | [post] self | type_tracker.rb:26:5:26:11 | self |
@ -415,6 +419,7 @@ trackEnd
| type_tracker.rb:30:20:30:20 | 4 | type_tracker.rb:27:10:27:11 | p2 |
| type_tracker.rb:30:20:30:20 | 4 | type_tracker.rb:30:20:30:20 | 4 |
| type_tracker.rb:30:20:30:20 | [post] 4 | type_tracker.rb:30:20:30:20 | [post] 4 |
| type_tracker.rb:31:1:31:21 | ** | type_tracker.rb:31:1:31:21 | ** |
| type_tracker.rb:31:1:31:21 | [post] self | type_tracker.rb:25:1:28:3 | self (keyword) |
| type_tracker.rb:31:1:31:21 | [post] self | type_tracker.rb:25:1:28:3 | self in keyword |
| type_tracker.rb:31:1:31:21 | [post] self | type_tracker.rb:26:5:26:11 | self |
@ -436,6 +441,7 @@ trackEnd
| type_tracker.rb:31:20:31:20 | 6 | type_tracker.rb:26:10:26:11 | p1 |
| type_tracker.rb:31:20:31:20 | 6 | type_tracker.rb:31:20:31:20 | 6 |
| type_tracker.rb:31:20:31:20 | [post] 6 | type_tracker.rb:31:20:31:20 | [post] 6 |
| type_tracker.rb:32:1:32:27 | ** | type_tracker.rb:32:1:32:27 | ** |
| type_tracker.rb:32:1:32:27 | [post] self | type_tracker.rb:32:1:32:27 | [post] self |
| type_tracker.rb:32:1:32:27 | call to keyword | type_tracker.rb:32:1:32:27 | call to keyword |
| type_tracker.rb:32:9:32:11 | :p2 | type_tracker.rb:32:9:32:11 | :p2 |

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

@ -12,6 +12,9 @@ edges
| CommandInjection.rb:46:15:46:26 | ...[...] : | CommandInjection.rb:50:24:50:36 | "echo #{...}" |
| CommandInjection.rb:64:18:64:23 | number : | CommandInjection.rb:65:14:65:29 | "echo #{...}" |
| CommandInjection.rb:72:23:72:33 | blah_number : | CommandInjection.rb:73:14:73:34 | "echo #{...}" |
| CommandInjection.rb:81:20:81:25 | **args : | CommandInjection.rb:82:22:82:25 | args : |
| CommandInjection.rb:82:22:82:25 | args : | CommandInjection.rb:82:22:82:37 | ...[...] : |
| CommandInjection.rb:82:22:82:37 | ...[...] : | CommandInjection.rb:82:14:82:39 | "echo #{...}" |
nodes
| CommandInjection.rb:6:15:6:20 | call to params : | semmle.label | call to params : |
| CommandInjection.rb:6:15:6:26 | ...[...] : | semmle.label | ...[...] : |
@ -30,6 +33,10 @@ nodes
| CommandInjection.rb:65:14:65:29 | "echo #{...}" | semmle.label | "echo #{...}" |
| CommandInjection.rb:72:23:72:33 | blah_number : | semmle.label | blah_number : |
| CommandInjection.rb:73:14:73:34 | "echo #{...}" | semmle.label | "echo #{...}" |
| CommandInjection.rb:81:20:81:25 | **args : | semmle.label | **args : |
| CommandInjection.rb:82:14:82:39 | "echo #{...}" | semmle.label | "echo #{...}" |
| CommandInjection.rb:82:22:82:25 | args : | semmle.label | args : |
| CommandInjection.rb:82:22:82:37 | ...[...] : | semmle.label | ...[...] : |
subpaths
#select
| CommandInjection.rb:7:10:7:15 | #{...} | CommandInjection.rb:6:15:6:20 | call to params : | CommandInjection.rb:7:10:7:15 | #{...} | This command depends on $@. | CommandInjection.rb:6:15:6:20 | call to params | a user-provided value |
@ -43,3 +50,4 @@ subpaths
| CommandInjection.rb:50:24:50:36 | "echo #{...}" | CommandInjection.rb:46:15:46:20 | call to params : | CommandInjection.rb:50:24:50:36 | "echo #{...}" | This command depends on $@. | CommandInjection.rb:46:15:46:20 | call to params | a user-provided value |
| CommandInjection.rb:65:14:65:29 | "echo #{...}" | CommandInjection.rb:64:18:64:23 | number : | CommandInjection.rb:65:14:65:29 | "echo #{...}" | This command depends on $@. | CommandInjection.rb:64:18:64:23 | number | a user-provided value |
| CommandInjection.rb:73:14:73:34 | "echo #{...}" | CommandInjection.rb:72:23:72:33 | blah_number : | CommandInjection.rb:73:14:73:34 | "echo #{...}" | This command depends on $@. | CommandInjection.rb:72:23:72:33 | blah_number | a user-provided value |
| CommandInjection.rb:82:14:82:39 | "echo #{...}" | CommandInjection.rb:81:20:81:25 | **args : | CommandInjection.rb:82:14:82:39 | "echo #{...}" | This command depends on $@. | CommandInjection.rb:81:20:81:25 | **args | a user-provided value |