Skip to content

Commit 9eda630

Browse files
RasmusWLRasmusWL
committed
Ruby: Add CallNode.getKeywordArgumentIncludeHashArgument 
1 parent 10968bf commit 9eda630

7 files changed

Lines changed: 46 additions & 93 deletions

File tree

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,46 @@ class CallNode extends LocalSourceNode, ExprNode {
8787

8888
/** Gets the block of this call. */
8989
Node getBlock() { result.asExpr() = node.getBlock() }
90+
91+
/**
92+
* Gets the data-flow node corresponding to the named argument of the call
93+
* corresponding to this data-flow node, also including values passed with (pre Ruby
94+
* 2.0) hash arguments.
95+
*
96+
* Such hash arguments are tracked back to their source location within functions, but
97+
* no inter-procedural analysis occurs.
98+
*
99+
* This means all 3 variants below will be handled by this predicate:
100+
*
101+
* ```ruby
102+
* foo(..., some_option: 42)
103+
* foo(..., { some_option: 42 })
104+
* options = { some_option: 42 }
105+
* foo(..., options)
106+
* ```
107+
*/
108+
Node getKeywordArgumentIncludeHashArgument(string name) {
109+
// to reduce number of computed tuples, I have put bindingset on both this and name,
110+
// meaning we only do the local backwards tracking for known calls and known names.
111+
// (not because a performance problem was seen, it just seemed right).
112+
result = this.getKeywordArgument(name)
113+
or
114+
exists(CfgNodes::ExprNodes::PairCfgNode pair |
115+
pair =
116+
this.getArgument(_)
117+
.getALocalSource()
118+
.asExpr()
119+
.(CfgNodes::ExprNodes::HashLiteralCfgNode)
120+
.getAKeyValuePair() and
121+
exprNode(pair.getKey())
122+
.getALocalSource()
123+
.asExpr()
124+
.getExpr()
125+
.getConstantValue()
126+
.isStringlikeValue(name) and
127+
result.asExpr() = pair.getValue()
128+
)
129+
}
90130
}
91131

92132
/**

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

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -66,24 +66,7 @@ class ExconHttpRequest extends HTTP::Client::Request::Range, DataFlow::CallNode
6666
DataFlow::Node getCertificateValidationControllingValue() {
6767
exists(DataFlow::CallNode newCall | newCall = connectionNode.getAValueReachableFromSource() |
6868
// Check for `ssl_verify_peer: false`
69-
result = newCall.getKeywordArgument("ssl_verify_peer")
70-
or
71-
// using a hashliteral
72-
exists(
73-
DataFlow::LocalSourceNode optionsNode, CfgNodes::ExprNodes::PairCfgNode p,
74-
DataFlow::Node key
75-
|
76-
// can't flow to argument 0, since that's the URL
77-
optionsNode.flowsTo(newCall.getArgument(any(int i | i > 0))) and
78-
p = optionsNode.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
79-
key.asExpr() = p.getKey() and
80-
key.getALocalSource()
81-
.asExpr()
82-
.getExpr()
83-
.getConstantValue()
84-
.isStringlikeValue("ssl_verify_peer") and
85-
result.asExpr() = p.getValue()
86-
)
69+
result = newCall.getKeywordArgumentIncludeHashArgument("ssl_verify_peer")
8770
)
8871
}
8972

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

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -60,20 +60,7 @@ class FaradayHttpRequest extends HTTP::Client::Request::Range, DataFlow::CallNod
6060
argName in ["verify", "verify_mode"] and
6161
exists(DataFlow::Node sslValue, DataFlow::CallNode newCall |
6262
newCall = connectionNode.getAValueReachableFromSource() and
63-
sslValue = newCall.getKeywordArgument("ssl")
64-
or
65-
// using a hashliteral
66-
exists(
67-
DataFlow::LocalSourceNode optionsNode, CfgNodes::ExprNodes::PairCfgNode p,
68-
DataFlow::Node key
69-
|
70-
// can't flow to argument 0, since that's the URL
71-
optionsNode.flowsTo(newCall.getArgument(any(int i | i > 0))) and
72-
p = optionsNode.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
73-
key.asExpr() = p.getKey() and
74-
key.getALocalSource().asExpr().getExpr().getConstantValue().isStringlikeValue("ssl") and
75-
sslValue.asExpr() = p.getValue()
76-
)
63+
sslValue = newCall.getKeywordArgumentIncludeHashArgument("ssl")
7764
|
7865
exists(CfgNodes::ExprNodes::PairCfgNode p, DataFlow::Node key |
7966
p = sslValue.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and

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

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -50,23 +50,7 @@ class HttpartyRequest extends HTTP::Client::Request::Range, DataFlow::CallNode {
5050

5151
/** Gets the value that controls certificate validation, if any. */
5252
DataFlow::Node getCertificateValidationControllingValue() {
53-
result = this.getKeywordArgument(["verify", "verify_peer"])
54-
or
55-
// using a hashliteral
56-
exists(
57-
DataFlow::LocalSourceNode optionsNode, CfgNodes::ExprNodes::PairCfgNode p, DataFlow::Node key
58-
|
59-
// can't flow to argument 0, since that's the URL
60-
optionsNode.flowsTo(this.getArgument(any(int i | i > 0))) and
61-
p = optionsNode.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
62-
key.asExpr() = p.getKey() and
63-
key.getALocalSource()
64-
.asExpr()
65-
.getExpr()
66-
.getConstantValue()
67-
.isStringlikeValue(["verify", "verify_peer"]) and
68-
result.asExpr() = p.getValue()
69-
)
53+
result = this.getKeywordArgumentIncludeHashArgument(["verify", "verify_peer"])
7054
}
7155

7256
override predicate disablesCertificateValidation(

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

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -39,22 +39,7 @@ class OpenUriRequest extends HTTP::Client::Request::Range, DataFlow::CallNode {
3939

4040
/** Gets the value that controls certificate validation, if any. */
4141
DataFlow::Node getCertificateValidationControllingValue() {
42-
result = this.getKeywordArgument("ssl_verify_mode")
43-
or
44-
// using a hashliteral
45-
exists(
46-
DataFlow::LocalSourceNode optionsNode, CfgNodes::ExprNodes::PairCfgNode p, DataFlow::Node key
47-
|
48-
optionsNode.flowsTo(this.getArgument(_)) and
49-
p = optionsNode.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
50-
key.asExpr() = p.getKey() and
51-
key.getALocalSource()
52-
.asExpr()
53-
.getExpr()
54-
.getConstantValue()
55-
.isStringlikeValue("ssl_verify_mode") and
56-
result.asExpr() = p.getValue()
57-
)
42+
result = this.getKeywordArgumentIncludeHashArgument("ssl_verify_mode")
5843
}
5944

6045
override predicate disablesCertificateValidation(

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

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -50,20 +50,7 @@ class RestClientHttpRequest extends HTTP::Client::Request::Range, DataFlow::Call
5050
/** Gets the value that controls certificate validation, if any. */
5151
DataFlow::Node getCertificateValidationControllingValue() {
5252
exists(DataFlow::CallNode newCall | newCall = connectionNode.getAValueReachableFromSource() |
53-
result = newCall.getKeywordArgument("verify_ssl")
54-
or
55-
// using a hashliteral
56-
exists(
57-
DataFlow::LocalSourceNode optionsNode, CfgNodes::ExprNodes::PairCfgNode p,
58-
DataFlow::Node key
59-
|
60-
// can't flow to argument 0, since that's the URL
61-
optionsNode.flowsTo(newCall.getArgument(any(int i | i > 0))) and
62-
p = optionsNode.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
63-
key.asExpr() = p.getKey() and
64-
key.getALocalSource().asExpr().getExpr().getConstantValue().isStringlikeValue("verify_ssl") and
65-
result.asExpr() = p.getValue()
66-
)
53+
result = newCall.getKeywordArgumentIncludeHashArgument("verify_ssl")
6754
)
6855
}
6956

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

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,7 @@ class TyphoeusHttpRequest extends HTTP::Client::Request::Range, DataFlow::CallNo
3131

3232
/** Gets the value that controls certificate validation, if any. */
3333
DataFlow::Node getCertificateValidationControllingValue() {
34-
result = this.getKeywordArgument("ssl_verifypeer")
35-
or
36-
// using a hashliteral
37-
exists(
38-
DataFlow::LocalSourceNode optionsNode, CfgNodes::ExprNodes::PairCfgNode p,
39-
DataFlow::Node key
40-
|
41-
// can't flow to argument 0, since that's the URL
42-
optionsNode.flowsTo(this.getArgument(any(int i | i > 0))) and
43-
p = optionsNode.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
44-
key.asExpr() = p.getKey() and
45-
key.getALocalSource().asExpr().getExpr().getConstantValue().isStringlikeValue("ssl_verifypeer") and
46-
result.asExpr() = p.getValue()
47-
)
34+
result = this.getKeywordArgumentIncludeHashArgument("ssl_verifypeer")
4835
}
4936

5037
override predicate disablesCertificateValidation(

0 commit comments

Comments
 (0)