Skip to content

Commit dd27ed8

Browse files
hvitvedhvitved
committed
Ruby: Desugar hash literals
```rb { a: 1, **splat, b: 2 } ``` becomes ```rb ::Hash.[](a: 1, **splat, b: 2) ```
1 parent 3943682 commit dd27ed8

File tree

17 files changed

+257
-99
lines changed

17 files changed

+257
-99
lines changed

‎ruby/ql/lib/codeql/ruby/ast/internal/Synthesis.qll‎

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -794,14 +794,15 @@ private module ArrayLiteralDesugar {
794794
exists(ArrayLiteral al |
795795
parent = al and
796796
i = -1 and
797-
child = SynthChild(MethodCallKind("[]", false, al.getNumberOfElements() + 1))
797+
child = SynthChild(MethodCallKind("[]", false, al.getNumberOfElements()))
798798
or
799-
exists(AstNode mc | mc = TMethodCallSynth(al, -1, _, _, _) |
800-
parent = mc and
799+
exists(AstNode mc |
800+
mc = TMethodCallSynth(al, -1, _, _, _) and
801+
parent = mc
802+
|
801803
i = 0 and
802804
child = SynthChild(ConstantReadAccessKind("::Array"))
803805
or
804-
parent = mc and
805806
child = childRef(al.getElement(i - 1))
806807
)
807808
)
@@ -825,13 +826,58 @@ private module ArrayLiteralDesugar {
825826
final override predicate methodCall(string name, boolean setter, int arity) {
826827
name = "[]" and
827828
setter = false and
828-
arity = any(ArrayLiteral al).getNumberOfElements() + 1
829+
arity = any(ArrayLiteral al).getNumberOfElements()
829830
}
830831

831832
final override predicate constantReadAccess(string name) { name = "::Array" }
832833
}
833834
}
834835

836+
private module HashLiteralDesugar {
837+
pragma[nomagic]
838+
private predicate hashLiteralSynthesis(AstNode parent, int i, Child child) {
839+
exists(HashLiteral hl |
840+
parent = hl and
841+
i = -1 and
842+
child = SynthChild(MethodCallKind("[]", false, hl.getNumberOfElements()))
843+
or
844+
exists(AstNode mc |
845+
mc = TMethodCallSynth(hl, -1, _, _, _) and
846+
parent = mc
847+
|
848+
i = 0 and
849+
child = SynthChild(ConstantReadAccessKind("::Hash"))
850+
or
851+
child = childRef(hl.getElement(i - 1))
852+
)
853+
)
854+
}
855+
856+
/**
857+
* ```rb
858+
* { a: 1, **splat, b: 2 }
859+
* ```
860+
* desugars to
861+
*
862+
* ```rb
863+
* ::Hash.[](a: 1, **splat, b: 2)
864+
* ```
865+
*/
866+
private class HashLiteralSynthesis extends Synthesis {
867+
final override predicate child(AstNode parent, int i, Child child) {
868+
hashLiteralSynthesis(parent, i, child)
869+
}
870+
871+
final override predicate methodCall(string name, boolean setter, int arity) {
872+
name = "[]" and
873+
setter = false and
874+
arity = any(HashLiteral hl).getNumberOfElements()
875+
}
876+
877+
final override predicate constantReadAccess(string name) { name = "::Hash" }
878+
}
879+
}
880+
835881
/**
836882
* ```rb
837883
* for x in xs

‎ruby/ql/lib/codeql/ruby/controlflow/CfgNodes.qll‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,9 @@ module ExprNodes {
484484
/** Gets the `n`th argument of this call. */
485485
final ExprCfgNode getArgument(int n) { e.hasCfgChild(e.getArgument(n), this, result) }
486486

487+
/** Gets an argument of this call. */
488+
final ExprCfgNode getAnArgument() { result = this.getArgument(_) }
489+
487490
/** Gets the the keyword argument whose key is `keyword` of this call. */
488491
final ExprCfgNode getKeywordArgument(string keyword) {
489492
exists(PairCfgNode n |
@@ -940,4 +943,39 @@ module ExprNodes {
940943

941944
final override ElementReference getExpr() { result = super.getExpr() }
942945
}
946+
947+
/**
948+
* A control-flow node that wraps an array literal. Array literals are desugared
949+
* into calls to `Array.[]`, so this includes both desugared calls as well as
950+
* explicit calls.
951+
*/
952+
class ArrayLiteralCfgNode extends MethodCallCfgNode {
953+
ArrayLiteralCfgNode() {
954+
exists(ConstantReadAccess array |
955+
array = this.getReceiver().getExpr() and
956+
e.(MethodCall).getMethodName() = "[]" and
957+
array.getName() = "Array" and
958+
array.hasGlobalScope()
959+
)
960+
}
961+
}
962+
963+
/**
964+
* A control-flow node that wraps a hash literal. Hash literals are desugared
965+
* into calls to `Hash.[]`, so this includes both desugared calls as well as
966+
* explicit calls.
967+
*/
968+
class HashLiteralCfgNode extends MethodCallCfgNode {
969+
HashLiteralCfgNode() {
970+
exists(ConstantReadAccess array |
971+
array = this.getReceiver().getExpr() and
972+
e.(MethodCall).getMethodName() = "[]" and
973+
array.getName() = "Hash" and
974+
array.hasGlobalScope()
975+
)
976+
}
977+
978+
/** Gets a pair of this hash literal. */
979+
PairCfgNode getAKeyValuePair() { result = this.getAnArgument() }
980+
}
943981
}

‎ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImpl.qll‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -986,10 +986,6 @@ module Trees {
986986

987987
private class GlobalVariableTree extends LeafTree, GlobalVariableAccess { }
988988

989-
private class HashLiteralTree extends StandardPostOrderTree, HashLiteral {
990-
final override ControlFlowTree getChildElement(int i) { result = this.getElement(i) }
991-
}
992-
993989
private class HashSplatNilParameterTree extends LeafTree, HashSplatNilParameter { }
994990

995991
private class HashSplatParameterTree extends NonDefaultValueParameterTree, HashSplatParameter { }

‎ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -278,16 +278,9 @@ private DataFlow::LocalSourceNode trackInstance(Module tp, TypeTracker t) {
278278
or
279279
result.asExpr().getExpr() instanceof StringlikeLiteral and tp = TResolved("String")
280280
or
281-
exists(ConstantReadAccess array, MethodCall mc |
282-
result.asExpr().getExpr() = mc and
283-
mc.getMethodName() = "[]" and
284-
mc.getReceiver() = array and
285-
array.getName() = "Array" and
286-
array.hasGlobalScope() and
287-
tp = TResolved("Array")
288-
)
281+
result.asExpr() instanceof CfgNodes::ExprNodes::ArrayLiteralCfgNode and tp = TResolved("Array")
289282
or
290-
result.asExpr().getExpr() instanceof HashLiteral and tp = TResolved("Hash")
283+
result.asExpr() instanceof CfgNodes::ExprNodes::HashLiteralCfgNode and tp = TResolved("Hash")
291284
or
292285
result.asExpr().getExpr() instanceof MethodBase and tp = TResolved("Symbol")
293286
or

‎ruby/ql/lib/codeql/ruby/frameworks/XmlParsing.qll‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,14 +52,15 @@ private class LibXmlRubyXmlParserCall extends XmlParserCall::Range, DataFlow::Ca
5252
override DataFlow::Node getInput() { result = this.getArgument(0) }
5353

5454
override predicate externalEntitiesEnabled() {
55-
exists(Pair pair |
56-
pair = this.getArgument(1).asExpr().getExpr().(HashLiteral).getAKeyValuePair() and
55+
exists(CfgNodes::ExprNodes::PairCfgNode pair |
56+
pair =
57+
this.getArgument(1).asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
5758
pair.getKey().getConstantValue().isStringOrSymbol("options") and
5859
pair.getValue() =
5960
[
6061
trackEnableFeature(TNOENT()), trackEnableFeature(TDTDLOAD()),
6162
trackDisableFeature(TNONET())
62-
].asExpr().getExpr()
63+
].asExpr()
6364
)
6465
}
6566
}

‎ruby/ql/lib/codeql/ruby/frameworks/http_clients/Excon.qll‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
private import ruby
2+
private import codeql.ruby.CFG
23
private import codeql.ruby.Concepts
34
private import codeql.ruby.ApiGraphs
45

@@ -91,16 +92,16 @@ class ExconHttpRequest extends HTTP::Client::Request::Range {
9192
predicate argSetsVerifyPeer(DataFlow::Node arg, boolean value, DataFlow::Node kvNode) {
9293
// Either passed as an individual key:value argument, e.g.:
9394
// Excon.get(..., ssl_verify_peer: false)
94-
isSslVerifyPeerPair(arg.asExpr().getExpr(), value) and
95+
isSslVerifyPeerPair(arg.asExpr(), value) and
9596
kvNode = arg
9697
or
9798
// Or as a single hash argument, e.g.:
9899
// Excon.get(..., { ssl_verify_peer: false, ... })
99-
exists(DataFlow::LocalSourceNode optionsNode, Pair p |
100-
p = optionsNode.asExpr().getExpr().(HashLiteral).getAKeyValuePair() and
100+
exists(DataFlow::LocalSourceNode optionsNode, CfgNodes::ExprNodes::PairCfgNode p |
101+
p = optionsNode.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
101102
isSslVerifyPeerPair(p, value) and
102103
optionsNode.flowsTo(arg) and
103-
kvNode.asExpr().getExpr() = p
104+
kvNode.asExpr() = p
104105
)
105106
}
106107

@@ -133,10 +134,10 @@ private predicate hasBooleanValue(DataFlow::Node node, boolean value) {
133134
}
134135

135136
/** Holds if `p` is the pair `ssl_verify_peer: <value>`. */
136-
private predicate isSslVerifyPeerPair(Pair p, boolean value) {
137+
private predicate isSslVerifyPeerPair(CfgNodes::ExprNodes::PairCfgNode p, boolean value) {
137138
exists(DataFlow::Node key, DataFlow::Node valueNode |
138-
key.asExpr().getExpr() = p.getKey() and valueNode.asExpr().getExpr() = p.getValue()
139-
|
139+
key.asExpr() = p.getKey() and
140+
valueNode.asExpr() = p.getValue() and
140141
isSslVerifyPeerLiteral(key) and
141142
hasBooleanValue(valueNode, value)
142143
)

‎ruby/ql/lib/codeql/ruby/frameworks/http_clients/Faraday.qll‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
private import ruby
2+
private import codeql.ruby.CFG
23
private import codeql.ruby.Concepts
34
private import codeql.ruby.ApiGraphs
45

@@ -56,16 +57,16 @@ class FaradayHttpRequest extends HTTP::Client::Request::Range {
5657
|
5758
// Either passed as an individual key:value argument, e.g.:
5859
// Faraday.new(..., ssl: {...})
59-
isSslOptionsPairDisablingValidation(arg.asExpr().getExpr()) and
60+
isSslOptionsPairDisablingValidation(arg.asExpr()) and
6061
disablingNode = arg
6162
or
6263
// Or as a single hash argument, e.g.:
6364
// Faraday.new(..., { ssl: {...} })
64-
exists(DataFlow::LocalSourceNode optionsNode, Pair p |
65-
p = optionsNode.asExpr().getExpr().(HashLiteral).getAKeyValuePair() and
65+
exists(DataFlow::LocalSourceNode optionsNode, CfgNodes::ExprNodes::PairCfgNode p |
66+
p = optionsNode.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
6667
isSslOptionsPairDisablingValidation(p) and
6768
optionsNode.flowsTo(arg) and
68-
disablingNode.asExpr().getExpr() = p
69+
disablingNode.asExpr() = p
6970
)
7071
)
7172
}
@@ -78,10 +79,10 @@ class FaradayHttpRequest extends HTTP::Client::Request::Range {
7879
* containing either `verify: false` or
7980
* `verify_mode: OpenSSL::SSL::VERIFY_NONE`.
8081
*/
81-
private predicate isSslOptionsPairDisablingValidation(Pair p) {
82+
private predicate isSslOptionsPairDisablingValidation(CfgNodes::ExprNodes::PairCfgNode p) {
8283
exists(DataFlow::Node key, DataFlow::Node value |
83-
key.asExpr().getExpr() = p.getKey() and value.asExpr().getExpr() = p.getValue()
84-
|
84+
key.asExpr() = p.getKey() and
85+
value.asExpr() = p.getValue() and
8586
isSymbolLiteral(key, "ssl") and
8687
(isHashWithVerifyFalse(value) or isHashWithVerifyModeNone(value))
8788
)
@@ -101,7 +102,7 @@ private predicate isSymbolLiteral(DataFlow::Node node, string valueText) {
101102
*/
102103
private predicate isHashWithVerifyFalse(DataFlow::Node node) {
103104
exists(DataFlow::LocalSourceNode hash |
104-
isVerifyFalsePair(hash.asExpr().getExpr().(HashLiteral).getAKeyValuePair()) and
105+
isVerifyFalsePair(hash.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair()) and
105106
hash.flowsTo(node)
106107
)
107108
}
@@ -112,7 +113,7 @@ private predicate isHashWithVerifyFalse(DataFlow::Node node) {
112113
*/
113114
private predicate isHashWithVerifyModeNone(DataFlow::Node node) {
114115
exists(DataFlow::LocalSourceNode hash |
115-
isVerifyModeNonePair(hash.asExpr().getExpr().(HashLiteral).getAKeyValuePair()) and
116+
isVerifyModeNonePair(hash.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair()) and
116117
hash.flowsTo(node)
117118
)
118119
}
@@ -121,10 +122,10 @@ private predicate isHashWithVerifyModeNone(DataFlow::Node node) {
121122
* Holds if the pair `p` has the key `:verify_mode` and the value
122123
* `OpenSSL::SSL::VERIFY_NONE`.
123124
*/
124-
private predicate isVerifyModeNonePair(Pair p) {
125+
private predicate isVerifyModeNonePair(CfgNodes::ExprNodes::PairCfgNode p) {
125126
exists(DataFlow::Node key, DataFlow::Node value |
126-
key.asExpr().getExpr() = p.getKey() and value.asExpr().getExpr() = p.getValue()
127-
|
127+
key.asExpr() = p.getKey() and
128+
value.asExpr() = p.getValue() and
128129
isSymbolLiteral(key, "verify_mode") and
129130
value = API::getTopLevelMember("OpenSSL").getMember("SSL").getMember("VERIFY_NONE").getAUse()
130131
)
@@ -133,10 +134,10 @@ private predicate isVerifyModeNonePair(Pair p) {
133134
/**
134135
* Holds if the pair `p` has the key `:verify` and the value `false`.
135136
*/
136-
private predicate isVerifyFalsePair(Pair p) {
137+
private predicate isVerifyFalsePair(CfgNodes::ExprNodes::PairCfgNode p) {
137138
exists(DataFlow::Node key, DataFlow::Node value |
138-
key.asExpr().getExpr() = p.getKey() and value.asExpr().getExpr() = p.getValue()
139-
|
139+
key.asExpr() = p.getKey() and
140+
value.asExpr() = p.getValue() and
140141
isSymbolLiteral(key, "verify") and
141142
isFalse(value)
142143
)

‎ruby/ql/lib/codeql/ruby/frameworks/http_clients/Httparty.qll‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
private import ruby
2+
private import codeql.ruby.CFG
23
private import codeql.ruby.Concepts
34
private import codeql.ruby.ApiGraphs
45

@@ -48,20 +49,21 @@ class HttpartyRequest extends HTTP::Client::Request::Range {
4849
// argument, and we're looking for `{ verify: false }` or
4950
// `{ verify_peer: false }`.
5051
exists(DataFlow::Node arg, int i |
51-
i > 0 and arg.asExpr().getExpr() = requestUse.asExpr().getExpr().(MethodCall).getArgument(i)
52+
i > 0 and
53+
arg.asExpr() = requestUse.asExpr().(CfgNodes::ExprNodes::MethodCallCfgNode).getArgument(i)
5254
|
5355
// Either passed as an individual key:value argument, e.g.:
5456
// HTTParty.get(..., verify: false)
55-
isVerifyFalsePair(arg.asExpr().getExpr()) and
57+
isVerifyFalsePair(arg.asExpr()) and
5658
disablingNode = arg
5759
or
5860
// Or as a single hash argument, e.g.:
5961
// HTTParty.get(..., { verify: false, ... })
60-
exists(DataFlow::LocalSourceNode optionsNode, Pair p |
61-
p = optionsNode.asExpr().getExpr().(HashLiteral).getAKeyValuePair() and
62+
exists(DataFlow::LocalSourceNode optionsNode, CfgNodes::ExprNodes::PairCfgNode p |
63+
p = optionsNode.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
6264
isVerifyFalsePair(p) and
6365
optionsNode.flowsTo(arg) and
64-
disablingNode.asExpr().getExpr() = p
66+
disablingNode.asExpr() = p
6567
)
6668
)
6769
}
@@ -88,10 +90,10 @@ private predicate isFalse(DataFlow::Node node) {
8890
/**
8991
* Holds if `p` is the pair `verify: false` or `verify_peer: false`.
9092
*/
91-
private predicate isVerifyFalsePair(Pair p) {
93+
private predicate isVerifyFalsePair(CfgNodes::ExprNodes::PairCfgNode p) {
9294
exists(DataFlow::Node key, DataFlow::Node value |
93-
key.asExpr().getExpr() = p.getKey() and value.asExpr().getExpr() = p.getValue()
94-
|
95+
key.asExpr() = p.getKey() and
96+
value.asExpr() = p.getValue() and
9597
isVerifyLiteral(key) and
9698
isFalse(value)
9799
)

0 commit comments

Comments
 (0)