Skip to content

Commit 2b74144

Browse files
alexrfordalexrford
authored
Merge pull request #13309 from maikypedia/maikypedia/ldap-injection
Ruby: Add LDAP Injection query
2 parents 4148798 + af85474 commit 2b74144

File tree

14 files changed

+477
-0
lines changed

14 files changed

+477
-0
lines changed

‎ruby/ql/lib/codeql/ruby/Concepts.qll‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1184,3 +1184,69 @@ module TemplateRendering {
11841184
abstract DataFlow::Node getTemplate();
11851185
}
11861186
}
1187+
1188+
/**
1189+
* A data-flow node that constructs a LDAP query.
1190+
*
1191+
* Often, it is worthy of an alert if an LDAP query is constructed such that
1192+
* executing it would be a security risk.
1193+
*
1194+
* If it is important that the query is executed, use `LdapExecution`.
1195+
*
1196+
* Extend this class to refine existing API models. If you want to model new APIs,
1197+
* extend `LdapConstruction::Range` instead.
1198+
*/
1199+
class LdapConstruction extends DataFlow::Node instanceof LdapConstruction::Range {
1200+
/** Gets the argument that specifies the query to be constructed. */
1201+
DataFlow::Node getQuery() { result = super.getQuery() }
1202+
}
1203+
1204+
/** Provides a class for modeling new LDAP query construction APIs. */
1205+
module LdapConstruction {
1206+
/**
1207+
* A data-flow node that constructs a LDAP query.
1208+
*
1209+
* Often, it is worthy of an alert if an LDAP query is constructed such that
1210+
* executing it would be a security risk.
1211+
*
1212+
* If it is important that the query is executed, use `LdapExecution`.
1213+
*
1214+
* Extend this class to model new APIs. If you want to refine existing API models,
1215+
* extend `LdapConstruction` instead.
1216+
*/
1217+
abstract class Range extends DataFlow::Node {
1218+
/** Gets the argument that specifies the query to be constructed. */
1219+
abstract DataFlow::Node getQuery();
1220+
}
1221+
}
1222+
1223+
/**
1224+
* A data-flow node that executes LDAP queries.
1225+
*
1226+
* If the context of interest is such that merely constructing a LDAP query
1227+
* would be valuable to report, consider using `LdapConstruction`.
1228+
*
1229+
* Extend this class to refine existing API models. If you want to model new APIs,
1230+
* extend `LdapExecution::Range` instead.
1231+
*/
1232+
class LdapExecution extends DataFlow::Node instanceof LdapExecution::Range {
1233+
/** Gets the argument that specifies the query to be executed. */
1234+
DataFlow::Node getQuery() { result = super.getQuery() }
1235+
}
1236+
1237+
/** Provides a class for modeling new LDAP query execution APIs. */
1238+
module LdapExecution {
1239+
/**
1240+
* A data-flow node that executes LDAP queries.
1241+
*
1242+
* If the context of interest is such that merely constructing a LDAP query
1243+
* would be valuable to report, consider using `LdapConstruction`.
1244+
*
1245+
* Extend this class to model new APIs. If you want to refine existing API models,
1246+
* extend `LdapExecution` instead.
1247+
*/
1248+
abstract class Range extends DataFlow::Node {
1249+
/** Gets the argument that specifies the query to be executed. */
1250+
abstract DataFlow::Node getQuery();
1251+
}
1252+
}

‎ruby/ql/lib/codeql/ruby/Frameworks.qll‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,3 +36,4 @@ private import codeql.ruby.frameworks.Mysql2
3636
private import codeql.ruby.frameworks.Pg
3737
private import codeql.ruby.frameworks.Yaml
3838
private import codeql.ruby.frameworks.Sequel
39+
private import codeql.ruby.frameworks.Ldap

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

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
/**
2+
* Provides modeling for `net-ldap` a ruby library for LDAP.
3+
*/
4+
5+
private import ruby
6+
private import codeql.ruby.ApiGraphs
7+
private import codeql.ruby.dataflow.FlowSummary
8+
private import codeql.ruby.Concepts
9+
10+
/**
11+
* Provides modeling for `net-ldap` a ruby library for LDAP.
12+
*/
13+
module NetLdap {
14+
/**
15+
* Flow summary for `Net::LDAP.new`. This method establishes a connection to a LDAP server.
16+
*/
17+
private class LdapConnSummary extends SummarizedCallable {
18+
LdapConnSummary() { this = "Net::LDAP.new" }
19+
20+
override MethodCall getACall() { result = any(NetLdapConnection l).asExpr().getExpr() }
21+
22+
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
23+
input = "Argument[0]" and output = "ReturnValue" and preservesValue = false
24+
}
25+
}
26+
27+
/**
28+
* Flow summary for `Net::LDAP.Filter`.
29+
*/
30+
private class LdapFilterSummary extends SummarizedCallable {
31+
LdapFilterSummary() { this = "Net::LDAP::Filter" }
32+
33+
override MethodCall getACall() { result = any(NetLdapFilter l).asExpr().getExpr() }
34+
35+
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
36+
input = ["Argument[0]", "Argument[1]"] and output = "ReturnValue" and preservesValue = false
37+
}
38+
}
39+
40+
/** Net LDAP Api Node */
41+
private API::Node ldap() { result = API::getTopLevelMember("Net").getMember("LDAP") }
42+
43+
/** A call that establishes a LDAP Connection */
44+
private class NetLdapConnection extends DataFlow::CallNode {
45+
NetLdapConnection() { this in [ldap().getAnInstantiation(), ldap().getAMethodCall("open")] }
46+
}
47+
48+
/** A call that constructs a LDAP query */
49+
private class NetLdapFilter extends LdapConstruction::Range, DataFlow::CallNode {
50+
NetLdapFilter() {
51+
this =
52+
any(ldap()
53+
.getMember("Filter")
54+
.getAMethodCall([
55+
"begins", "bineq", "contains", "ends", "eq", "equals", "ex", "ge", "le", "ne",
56+
"present"
57+
])
58+
)
59+
}
60+
61+
override DataFlow::Node getQuery() { result = this.getArgument([0, 1]) }
62+
}
63+
64+
/** A call considered as a LDAP execution. */
65+
private class NetLdapExecution extends LdapExecution::Range, DataFlow::CallNode {
66+
NetLdapExecution() { this = any(NetLdapConnection l).getAMethodCall("search") }
67+
68+
override DataFlow::Node getQuery() { result = this.getKeywordArgument(_) }
69+
}
70+
}

‎ruby/ql/lib/codeql/ruby/security/LdapInjectionCustomizations.qll‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
/**
2+
* Provides default sources, sinks and sanitizers for detecting
3+
* LDAP Injections, as well as extension points for adding your own
4+
*/
5+
6+
private import codeql.ruby.Concepts
7+
private import codeql.ruby.DataFlow
8+
private import codeql.ruby.dataflow.BarrierGuards
9+
private import codeql.ruby.dataflow.RemoteFlowSources
10+
private import codeql.ruby.ApiGraphs
11+
12+
/**
13+
* Provides default sources, sinks and sanitizers for detecting
14+
* LDAP Injections, as well as extension points for adding your own
15+
*/
16+
module LdapInjection {
17+
/** A data flow source for LDAP Injection vulnerabilities */
18+
abstract class Source extends DataFlow::Node { }
19+
20+
/** A data flow sink for LDAP Injection vulnerabilities */
21+
abstract class Sink extends DataFlow::Node { }
22+
23+
/** A sanitizer for LDAP Injection vulnerabilities. */
24+
abstract class Sanitizer extends DataFlow::Node { }
25+
26+
/**
27+
* Additional taint steps for "LDAP Injection" vulnerabilities.
28+
*/
29+
predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
30+
attributeArrayTaintStep(nodeFrom, nodeTo)
31+
}
32+
33+
/**
34+
* Additional taint step to handle elements inside an array,
35+
* specifically in the context of the following LDAP search function:
36+
*
37+
* ldap.search(base: "", filter: "", attributes: [name])
38+
*/
39+
private predicate attributeArrayTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
40+
exists(DataFlow::CallNode filterCall |
41+
filterCall.getMethodName() = "[]" and
42+
n1 = filterCall.getArgument(_) and
43+
n2 = filterCall
44+
)
45+
}
46+
47+
/**
48+
* A source of remote user input, considered as a flow source.
49+
*/
50+
private class RemoteFlowSourceAsSource extends Source, RemoteFlowSource { }
51+
52+
/**
53+
* An LDAP query execution considered as a flow sink.
54+
*/
55+
private class LdapExecutionAsSink extends Sink {
56+
LdapExecutionAsSink() { this = any(LdapExecution l).getQuery() }
57+
}
58+
59+
/**
60+
* A comparison with a constant string, considered as a sanitizer-guard.
61+
*/
62+
private class StringConstCompareAsSanitizerGuard extends Sanitizer, StringConstCompareBarrier { }
63+
64+
/**
65+
* An inclusion check against an array of constant strings, considered as a
66+
* sanitizer-guard.
67+
*/
68+
private class StringConstArrayInclusionCallAsSanitizer extends Sanitizer,
69+
StringConstArrayInclusionCallBarrier
70+
{ }
71+
72+
/**
73+
* A call to `Net::LDAP::Filter.escape`, considered as a sanitizer.
74+
*/
75+
class NetLdapFilterEscapeSanitization extends Sanitizer {
76+
NetLdapFilterEscapeSanitization() {
77+
this =
78+
API::getTopLevelMember("Net").getMember("LDAP").getMember("Filter").getAMethodCall("escape")
79+
}
80+
}
81+
}

‎ruby/ql/lib/codeql/ruby/security/LdapInjectionQuery.qll‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
* Provides default sources, sinks and sanitizers for detecting
3+
* LDAP Injections, as well as extension points for adding your own
4+
*/
5+
6+
private import codeql.ruby.DataFlow
7+
private import codeql.ruby.TaintTracking
8+
9+
/** Provides a taint-tracking configuration for detecting LDAP Injections vulnerabilities. */
10+
module LdapInjection {
11+
import LdapInjectionCustomizations::LdapInjection
12+
13+
/**
14+
* A taint-tracking configuration for detecting LDAP Injections vulnerabilities.
15+
*/
16+
private module Config implements DataFlow::ConfigSig {
17+
predicate isSource(DataFlow::Node source) { source instanceof Source }
18+
19+
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
20+
21+
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
22+
23+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
24+
LdapInjection::isAdditionalFlowStep(node1, node2)
25+
}
26+
}
27+
28+
import TaintTracking::Global<Config>
29+
}

‎ruby/ql/src/change-notes/2023-05-28-ldap-injection-query.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* Added a new experimental query, `rb/ldap-injection`, to detect cases where user input is incorporated into LDAP queries without proper validation or sanitization, potentially leading to LDAP injection vulnerabilities.

‎ruby/ql/src/experimental/ldap-injection/LdapInjection.qhelp‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
If an LDAP query or DN is built using string concatenation or string formatting, and the
9+
components of the concatenation include user input without any proper sanitization, a user
10+
is likely to be able to run malicious LDAP queries.
11+
</p>
12+
</overview>
13+
14+
<recommendation>
15+
<p>
16+
If user input must be included in an LDAP query or DN, it should be escaped to
17+
avoid a malicious user providing special characters that change the meaning
18+
of the query.
19+
</p>
20+
</recommendation>
21+
22+
<example>
23+
<p>
24+
In the following Rails example, an <code>ActionController</code> class
25+
has a <code>ldap_handler</code> method to handle requests.
26+
</p>
27+
28+
<p>
29+
The user and dc is specified using a parameter, <code>user_name</code> and <code>dc</code> provided by
30+
the client which it then uses to build a LDAP query and DN. This value is accessible using the <code>params</code> method.
31+
</p>
32+
33+
<p>The first example uses the unsanitized user input directly
34+
in the search filter and DN for the LDAP query.
35+
A malicious user could provide special characters to change the meaning of these
36+
components, and search for a completely different set of values.</p>
37+
38+
<sample src="examples/LdapInjectionBad.rb" />
39+
40+
<p>In the second example, the input provided by the user is sanitized before it is included in the search filter or DN.
41+
This ensures the meaning of the query cannot be changed by a malicious user.</p>
42+
43+
<sample src="examples/LdapInjectionGood.rb" />
44+
</example>
45+
46+
<references>
47+
<li>OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/LDAP_Injection_Prevention_Cheat_Sheet.html">LDAP Injection Prevention Cheat Sheet</a>.</li>
48+
<li>OWASP: <a href="https://owasp.org/www-community/attacks/LDAP_Injection">LDAP Injection</a>.</li>
49+
<li>Wikipedia: <a href="https://en.wikipedia.org/wiki/LDAP_injection">LDAP injection</a>.</li>
50+
<li>BlackHat: <a href="https://www.blackhat.com/presentations/bh-europe-08/Alonso-Parada/Whitepaper/bh-eu-08-alonso-parada-WP.pdf">LDAP Injection and Blind LDAP Injection</a>.</li>
51+
<li>LDAP: <a href="https://ldap.com/2018/05/04/understanding-and-defending-against-ldap-injection-attacks/">Understanding and Defending Against LDAP Injection Attacks</a>.</li>
52+
</references>
53+
</qhelp>

‎ruby/ql/src/experimental/ldap-injection/LdapInjection.ql‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* @name LDAP Injection
3+
* @description Building an LDAP query from user-controlled sources is vulnerable to insertion of
4+
* malicious LDAP code by the user.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @security-severity 9.8
8+
* @precision high
9+
* @id rb/ldap-injection
10+
* @tags security
11+
* external/cwe/cwe-090
12+
*/
13+
14+
import codeql.ruby.DataFlow
15+
import codeql.ruby.security.LdapInjectionQuery
16+
import LdapInjection::PathGraph
17+
18+
from LdapInjection::PathNode source, LdapInjection::PathNode sink
19+
where LdapInjection::flowPath(source, sink)
20+
select sink.getNode(), source, sink, "This LDAP query depends on a $@.", source.getNode(),
21+
"user-provided value"

‎ruby/ql/src/experimental/ldap-injection/examples/LdapInjectionBad.rb‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
require 'net/ldap'
2+
3+
class BadLdapController < ActionController::Base
4+
def ldap_handler
5+
name = params[:user_name]
6+
dc = params[:dc]
7+
ldap = Net::LDAP.new(
8+
host: 'ldap.example.com',
9+
port: 636,
10+
encryption: :simple_tls,
11+
auth: {
12+
method: :simple,
13+
username: 'uid=admin,dc=example,dc=com',
14+
password: 'adminpassword'
15+
}
16+
)
17+
filter = Net::LDAP::Filter.eq('foo', name)
18+
attrs = [name]
19+
result = ldap.search(base: "ou=people,dc=#{dc},dc=com", filter: filter, attributes: attrs)
20+
end
21+
end

‎ruby/ql/src/experimental/ldap-injection/examples/LdapInjectionGood.rb‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
require 'net/ldap'
2+
3+
class GoodLdapController < ActionController::Base
4+
def ldap_handler
5+
name = params[:user_name]
6+
ldap = Net::LDAP.new(
7+
host: 'ldap.example.com',
8+
port: 636,
9+
encryption: :simple_tls,
10+
auth: {
11+
method: :simple,
12+
username: 'uid=admin,dc=example,dc=com',
13+
password: 'adminpassword'
14+
}
15+
)
16+
17+
name = if ["admin", "guest"].include? name
18+
name
19+
else
20+
name = "none"
21+
end
22+
23+
filter = Net::LDAP::Filter.eq('foo', name)
24+
attrs = ['dn']
25+
result = ldap.search(base: 'ou=people,dc=example,dc=com', filter: filter, attributes: attrs)
26+
end
27+
end

0 commit comments

Comments
 (0)