Skip to content

Commit 0ac3624

Browse files
RasmusWLRasmusWL
committed
Ruby: Implement new disablesCertificateValidation for all HTTP client models
Sadly most alert text changed, but the two important changes are: 1. The request on RestClient.rb:19 now has an expanded alert text, highlighting where the origin of the value that disables certificate validation comes from. (in this case, it's trivial since it's the line right above) 2. We handle passing `false`/`OpenSSL::SSL::VERIFY_NONE` the same in the argument passing examples in Faraday.rb
1 parent 1f028ac commit 0ac3624

File tree

9 files changed

+301
-353
lines changed

9 files changed

+301
-353
lines changed

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

Lines changed: 69 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ private import codeql.ruby.CFG
77
private import codeql.ruby.Concepts
88
private import codeql.ruby.ApiGraphs
99
private import codeql.ruby.DataFlow
10+
private import codeql.ruby.dataflow.internal.DataFlowImplForLibraries as DataFlowImplForLibraries
1011

1112
/**
1213
* A call that makes an HTTP request using `Excon`.
@@ -61,96 +62,87 @@ class ExconHttpRequest extends HTTP::Client::Request::Range, DataFlow::CallNode
6162
result = connectionUse.(DataFlow::CallNode).getArgument(0)
6263
}
6364

64-
override predicate disablesCertificateValidation(DataFlow::Node disablingNode) {
65-
// Check for `ssl_verify_peer: false` in the options hash.
66-
exists(DataFlow::Node arg, int i |
67-
i > 0 and
68-
arg = connectionNode.getAValueReachableFromSource().(DataFlow::CallNode).getArgument(i)
69-
|
70-
argSetsVerifyPeer(arg, false, disablingNode)
71-
)
72-
or
73-
// Or we see a call to `Excon.defaults[:ssl_verify_peer] = false` before the
74-
// request, and no `ssl_verify_peer: true` in the explicit options hash for
75-
// the request call.
76-
exists(DataFlow::CallNode disableCall |
77-
setsDefaultVerification(disableCall, false) and
78-
disableCall.asExpr().getASuccessor+() = this.asExpr() and
79-
disablingNode = disableCall and
80-
not exists(DataFlow::Node arg, int i |
81-
i > 0 and
82-
arg = connectionNode.getAValueReachableFromSource().(DataFlow::CallNode).getArgument(i)
65+
/** Gets the value that controls certificate validation, if any. */
66+
DataFlow::Node getCertificateValidationControllingValue() {
67+
exists(DataFlow::CallNode newCall | newCall = connectionNode.getAValueReachableFromSource() |
68+
// 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
8375
|
84-
argSetsVerifyPeer(arg, true, _)
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()
8586
)
8687
)
8788
}
8889

8990
override predicate disablesCertificateValidation(
9091
DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
9192
) {
92-
disablesCertificateValidation(disablingNode) and
93-
argumentOrigin = disablingNode
93+
any(ExconDisablesCertificateValidationConfiguration config)
94+
.hasFlow(argumentOrigin, disablingNode) and
95+
disablingNode = this.getCertificateValidationControllingValue()
96+
or
97+
// We set `Excon.defaults[:ssl_verify_peer]` or `Excon.ssl_verify_peer` = false`
98+
// before the request, and no `ssl_verify_peer: true` in the explicit options hash
99+
// for the request call.
100+
exists(DataFlow::CallNode disableCall, BooleanLiteral value |
101+
// Excon.defaults[:ssl_verify_peer]
102+
disableCall = API::getTopLevelMember("Excon").getReturn("defaults").getAMethodCall("[]=") and
103+
disableCall
104+
.getArgument(0)
105+
.getALocalSource()
106+
.asExpr()
107+
.getExpr()
108+
.getConstantValue()
109+
.isStringlikeValue("ssl_verify_peer") and
110+
disablingNode = disableCall.getArgument(1) and
111+
argumentOrigin = disablingNode.getALocalSource() and
112+
value = argumentOrigin.asExpr().getExpr()
113+
or
114+
// Excon.ssl_verify_peer
115+
disableCall = API::getTopLevelMember("Excon").getAMethodCall("ssl_verify_peer=") and
116+
disablingNode = disableCall.getArgument(0) and
117+
argumentOrigin = disablingNode.getALocalSource() and
118+
value = argumentOrigin.asExpr().getExpr()
119+
|
120+
value.getValue() = false and
121+
disableCall.asExpr().getASuccessor+() = this.asExpr() and
122+
// no `ssl_verify_peer: true` in the request call.
123+
not this.getCertificateValidationControllingValue()
124+
.getALocalSource()
125+
.asExpr()
126+
.getExpr()
127+
.(BooleanLiteral)
128+
.getValue() = true
129+
)
94130
}
95131

96132
override string getFramework() { result = "Excon" }
97133
}
98134

99-
/**
100-
* Holds if `arg` represents an options hash that contains the key
101-
* `:ssl_verify_peer` with `value`, where `kvNode` is the data-flow node for
102-
* this key-value pair.
103-
*/
104-
predicate argSetsVerifyPeer(DataFlow::Node arg, boolean value, DataFlow::Node kvNode) {
105-
// Either passed as an individual key:value argument, e.g.:
106-
// Excon.get(..., ssl_verify_peer: false)
107-
isSslVerifyPeerPair(arg.asExpr(), value) and
108-
kvNode = arg
109-
or
110-
// Or as a single hash argument, e.g.:
111-
// Excon.get(..., { ssl_verify_peer: false, ... })
112-
exists(DataFlow::LocalSourceNode optionsNode, CfgNodes::ExprNodes::PairCfgNode p |
113-
p = optionsNode.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
114-
isSslVerifyPeerPair(p, value) and
115-
optionsNode.flowsTo(arg) and
116-
kvNode.asExpr() = p
117-
)
118-
}
119-
120-
/**
121-
* Holds if `callNode` sets `Excon.defaults[:ssl_verify_peer]` or
122-
* `Excon.ssl_verify_peer` to `value`.
123-
*/
124-
private predicate setsDefaultVerification(DataFlow::CallNode callNode, boolean value) {
125-
callNode = API::getTopLevelMember("Excon").getReturn("defaults").getAMethodCall("[]=") and
126-
isSslVerifyPeerLiteral(callNode.getArgument(0)) and
127-
hasBooleanValue(callNode.getArgument(1), value)
128-
or
129-
callNode = API::getTopLevelMember("Excon").getAMethodCall("ssl_verify_peer=") and
130-
hasBooleanValue(callNode.getArgument(0), value)
131-
}
132-
133-
private predicate isSslVerifyPeerLiteral(DataFlow::Node node) {
134-
exists(DataFlow::LocalSourceNode literal |
135-
literal.asExpr().getExpr().getConstantValue().isStringlikeValue("ssl_verify_peer") and
136-
literal.flowsTo(node)
137-
)
138-
}
135+
/** A configuration to track values that can disable certificate validation for Excon. */
136+
private class ExconDisablesCertificateValidationConfiguration extends DataFlowImplForLibraries::Configuration {
137+
ExconDisablesCertificateValidationConfiguration() {
138+
this = "ExconDisablesCertificateValidationConfiguration"
139+
}
139140

140-
/** Holds if `node` can contain `value`. */
141-
private predicate hasBooleanValue(DataFlow::Node node, boolean value) {
142-
exists(DataFlow::LocalSourceNode literal |
143-
literal.asExpr().getExpr().(BooleanLiteral).getValue() = value and
144-
literal.flowsTo(node)
145-
)
146-
}
141+
override predicate isSource(DataFlow::Node source) {
142+
source.asExpr().getExpr().(BooleanLiteral).isFalse()
143+
}
147144

148-
/** Holds if `p` is the pair `ssl_verify_peer: <value>`. */
149-
private predicate isSslVerifyPeerPair(CfgNodes::ExprNodes::PairCfgNode p, boolean value) {
150-
exists(DataFlow::Node key, DataFlow::Node valueNode |
151-
key.asExpr() = p.getKey() and
152-
valueNode.asExpr() = p.getValue() and
153-
isSslVerifyPeerLiteral(key) and
154-
hasBooleanValue(valueNode, value)
155-
)
145+
override predicate isSink(DataFlow::Node sink) {
146+
sink = any(ExconHttpRequest req).getCertificateValidationControllingValue()
147+
}
156148
}

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

Lines changed: 42 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ private import codeql.ruby.CFG
77
private import codeql.ruby.Concepts
88
private import codeql.ruby.ApiGraphs
99
private import codeql.ruby.DataFlow
10+
private import codeql.ruby.dataflow.internal.DataFlowImplForLibraries as DataFlowImplForLibraries
1011

1112
/**
1213
* A call that makes an HTTP request using `Faraday`.
@@ -49,119 +50,66 @@ class FaradayHttpRequest extends HTTP::Client::Request::Range, DataFlow::CallNod
4950
result = connectionUse.(DataFlow::CallNode).getKeywordArgument("url")
5051
}
5152

52-
override predicate disablesCertificateValidation(DataFlow::Node disablingNode) {
53+
/** Gets the value that controls certificate validation, if any, with argument name `name`. */
54+
DataFlow::Node getCertificateValidationControllingValue(string argName) {
5355
// `Faraday::new` takes an options hash as its second argument, and we're
5456
// looking for
5557
// `{ ssl: { verify: false } }`
5658
// or
5759
// `{ ssl: { verify_mode: OpenSSL::SSL::VERIFY_NONE } }`
58-
exists(DataFlow::Node arg, int i |
59-
i > 0 and
60-
arg = connectionNode.getAValueReachableFromSource().(DataFlow::CallNode).getArgument(i)
61-
|
62-
// Either passed as an individual key:value argument, e.g.:
63-
// Faraday.new(..., ssl: {...})
64-
isSslOptionsPairDisablingValidation(arg.asExpr()) and
65-
disablingNode = arg
60+
argName in ["verify", "verify_mode"] and
61+
exists(DataFlow::Node sslValue, DataFlow::CallNode newCall |
62+
newCall = connectionNode.getAValueReachableFromSource() and
63+
sslValue = newCall.getKeywordArgument("ssl")
6664
or
67-
// Or as a single hash argument, e.g.:
68-
// Faraday.new(..., { ssl: {...} })
69-
exists(DataFlow::LocalSourceNode optionsNode, CfgNodes::ExprNodes::PairCfgNode p |
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
7072
p = optionsNode.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
71-
isSslOptionsPairDisablingValidation(p) and
72-
optionsNode.flowsTo(arg) and
73-
disablingNode.asExpr() = p
73+
key.asExpr() = p.getKey() and
74+
key.getALocalSource().asExpr().getExpr().getConstantValue().isStringlikeValue("ssl") and
75+
sslValue.asExpr() = p.getValue()
76+
)
77+
|
78+
exists(CfgNodes::ExprNodes::PairCfgNode p, DataFlow::Node key |
79+
p = sslValue.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair() and
80+
key.asExpr() = p.getKey() and
81+
key.getALocalSource().asExpr().getExpr().getConstantValue().isStringlikeValue(argName) and
82+
result.asExpr() = p.getValue()
7483
)
7584
)
7685
}
7786

7887
override predicate disablesCertificateValidation(
7988
DataFlow::Node disablingNode, DataFlow::Node argumentOrigin
8089
) {
81-
disablesCertificateValidation(disablingNode) and
82-
argumentOrigin = disablingNode
90+
any(FaradayDisablesCertificateValidationConfiguration config)
91+
.hasFlow(argumentOrigin, disablingNode) and
92+
disablingNode = this.getCertificateValidationControllingValue(_)
8393
}
8494

8595
override string getFramework() { result = "Faraday" }
8696
}
8797

88-
/**
89-
* Holds if the pair `p` contains the key `:ssl` for which the value is a hash
90-
* containing either `verify: false` or
91-
* `verify_mode: OpenSSL::SSL::VERIFY_NONE`.
92-
*/
93-
private predicate isSslOptionsPairDisablingValidation(CfgNodes::ExprNodes::PairCfgNode p) {
94-
exists(DataFlow::Node key, DataFlow::Node value |
95-
key.asExpr() = p.getKey() and
96-
value.asExpr() = p.getValue() and
97-
isSymbolLiteral(key, "ssl") and
98-
(isHashWithVerifyFalse(value) or isHashWithVerifyModeNone(value))
99-
)
100-
}
101-
102-
/** Holds if `node` represents the symbol literal with the given `valueText`. */
103-
private predicate isSymbolLiteral(DataFlow::Node node, string valueText) {
104-
exists(DataFlow::LocalSourceNode literal |
105-
literal.asExpr().getExpr().getConstantValue().isStringlikeValue(valueText) and
106-
literal.flowsTo(node)
107-
)
108-
}
109-
110-
/**
111-
* Holds if `node` represents a hash containing the key-value pair
112-
* `verify: false`.
113-
*/
114-
private predicate isHashWithVerifyFalse(DataFlow::Node node) {
115-
exists(DataFlow::LocalSourceNode hash |
116-
isVerifyFalsePair(hash.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair()) and
117-
hash.flowsTo(node)
118-
)
119-
}
120-
121-
/**
122-
* Holds if `node` represents a hash containing the key-value pair
123-
* `verify_mode: OpenSSL::SSL::VERIFY_NONE`.
124-
*/
125-
private predicate isHashWithVerifyModeNone(DataFlow::Node node) {
126-
exists(DataFlow::LocalSourceNode hash |
127-
isVerifyModeNonePair(hash.asExpr().(CfgNodes::ExprNodes::HashLiteralCfgNode).getAKeyValuePair()) and
128-
hash.flowsTo(node)
129-
)
130-
}
131-
132-
/**
133-
* Holds if the pair `p` has the key `:verify_mode` and the value
134-
* `OpenSSL::SSL::VERIFY_NONE`.
135-
*/
136-
private predicate isVerifyModeNonePair(CfgNodes::ExprNodes::PairCfgNode p) {
137-
exists(DataFlow::Node key, DataFlow::Node value |
138-
key.asExpr() = p.getKey() and
139-
value.asExpr() = p.getValue() and
140-
isSymbolLiteral(key, "verify_mode") and
141-
value =
142-
API::getTopLevelMember("OpenSSL")
143-
.getMember("SSL")
144-
.getMember("VERIFY_NONE")
145-
.getAValueReachableFromSource()
146-
)
147-
}
98+
/** A configuration to track values that can disable certificate validation for Faraday. */
99+
private class FaradayDisablesCertificateValidationConfiguration extends DataFlowImplForLibraries::Configuration {
100+
FaradayDisablesCertificateValidationConfiguration() {
101+
this = "FaradayDisablesCertificateValidationConfiguration"
102+
}
148103

149-
/**
150-
* Holds if the pair `p` has the key `:verify` and the value `false`.
151-
*/
152-
private predicate isVerifyFalsePair(CfgNodes::ExprNodes::PairCfgNode p) {
153-
exists(DataFlow::Node key, DataFlow::Node value |
154-
key.asExpr() = p.getKey() and
155-
value.asExpr() = p.getValue() and
156-
isSymbolLiteral(key, "verify") and
157-
isFalse(value)
158-
)
159-
}
104+
override predicate isSource(DataFlow::Node source, DataFlowImplForLibraries::FlowState state) {
105+
source.asExpr().getExpr().(BooleanLiteral).isFalse() and
106+
state = "verify"
107+
or
108+
source = API::getTopLevelMember("OpenSSL").getMember("SSL").getMember("VERIFY_NONE").asSource() and
109+
state = "verify_mode"
110+
}
160111

161-
/** Holds if `node` can contain the Boolean value `false`. */
162-
private predicate isFalse(DataFlow::Node node) {
163-
exists(DataFlow::LocalSourceNode literal |
164-
literal.asExpr().getExpr().(BooleanLiteral).isFalse() and
165-
literal.flowsTo(node)
166-
)
112+
override predicate isSink(DataFlow::Node sink, DataFlowImplForLibraries::FlowState state) {
113+
sink = any(FaradayHttpRequest req).getCertificateValidationControllingValue(state)
114+
}
167115
}

0 commit comments

Comments
 (0)