mirror of
https://github.com/github/codeql.git
synced 2026-05-03 04:39:29 +02:00
naively move ldap into the SQL injection query
This commit is contained in:
Erik Krogh Kristensen
@@ -99,6 +99,7 @@ import semmle.javascript.frameworks.History
|
||||
import semmle.javascript.frameworks.Immutable
|
||||
import semmle.javascript.frameworks.Knex
|
||||
import semmle.javascript.frameworks.LazyCache
|
||||
import semmle.javascript.frameworks.Ldapjs
|
||||
import semmle.javascript.frameworks.LodashUnderscore
|
||||
import semmle.javascript.frameworks.Logging
|
||||
import semmle.javascript.frameworks.HttpFrameworks
|
||||
|
||||
@@ -41,4 +41,44 @@ module SqlInjection {
|
||||
class GraphqlInjectionSink extends Sink {
|
||||
GraphqlInjectionSink() { this instanceof GraphQL::GraphQLString }
|
||||
}
|
||||
|
||||
/**
|
||||
* An LDAP filter for an API call that executes an operation against the LDAP server.
|
||||
*/
|
||||
class LdapjsSearchFilterAsSink extends Sink {
|
||||
// TODO: As taint-step?
|
||||
/*
|
||||
* override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
|
||||
* exists(LdapjsParseFilter filter |
|
||||
* pred = filter.getArgument(0) and
|
||||
* succ = filter
|
||||
* )
|
||||
* }
|
||||
*/
|
||||
|
||||
LdapjsSearchFilterAsSink() { this instanceof Ldapjs::LdapjsSearchFilter }
|
||||
}
|
||||
|
||||
/**
|
||||
* An LDAP DN argument for an API call that executes an operation against the LDAP server.
|
||||
*/
|
||||
class LdapjsDNArgumentAsSink extends Sink {
|
||||
LdapjsDNArgumentAsSink() { this instanceof Ldapjs::LdapjsDNArgument }
|
||||
}
|
||||
|
||||
/**
|
||||
* A call to a function whose name suggests that it escapes LDAP search query parameter.
|
||||
*/
|
||||
class FilterOrDNSanitizationCall extends Sanitizer, DataFlow::CallNode {
|
||||
// TODO: remove, or use something else? (AdhocWhitelistSanitizer?)
|
||||
FilterOrDNSanitizationCall() {
|
||||
exists(string sanitize, string input |
|
||||
sanitize = "(?:escape|saniti[sz]e|validate|filter)" and
|
||||
input = "[Ii]nput?"
|
||||
|
|
||||
this.getCalleeName()
|
||||
.regexpMatch("(?i)(" + sanitize + input + ")" + "|(" + input + sanitize + ")")
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,50 +0,0 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<overview>
|
||||
<p>If an LDAP query is built using string concatenation or string formatting, and the
|
||||
components of the concatenation include user input without any proper sanitization, a user
|
||||
is likely to be able to run malicious LDAP queries.</p>
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>If user input must be included in an LDAP query, it should be escaped to
|
||||
avoid a malicious user providing special characters that change the meaning
|
||||
of the query. In NodeJS, it is possible to build the LDAP query using frameworks like <code>ldapjs</code>.
|
||||
The library provides a <code>Filter API</code>, however it's still possibile to pass a string version of an LDAP filter.
|
||||
A good practice is to escape filter characters that could change the meaning of the query (https://tools.ietf.org/search/rfc4515#section-3).</p>
|
||||
</recommendation>
|
||||
|
||||
<example>
|
||||
<p>In the following examples, the code accepts a <code>username</code> from the user, which it uses in a LDAP query.</p>
|
||||
|
||||
<p>The first and the second example uses the unsanitized user input directly
|
||||
in the search filter for the LDAP query.
|
||||
A malicious user could provide special characters to change the meaning of these
|
||||
queries, and search for a completely different set of values.
|
||||
</p>
|
||||
<sample src="examples/example_bad1.js" />
|
||||
<sample src="examples/example_bad2.js" />
|
||||
|
||||
|
||||
<p>In the third example the <code>username</code> is sanitized before it is included in the search filters.
|
||||
This ensures the meaning of the query cannot be changed by a malicious user.</p>
|
||||
|
||||
<sample src="examples/example_good1.js" />
|
||||
|
||||
<p>In the fourth example the <code>username</code> is passed to an <code>OrFilter</code> filter before it is included in the search filters.
|
||||
This ensures the meaning of the query cannot be changed by a malicious user.</p>
|
||||
|
||||
<sample src="examples/example_good2.js" />
|
||||
</example>
|
||||
|
||||
<references>
|
||||
<li>OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/LDAP_Injection_Prevention_Cheat_Sheet.html">LDAP Injection Prevention Cheat Sheet</a>.</li>
|
||||
<li>LDAPjs: <a href="http://ldapjs.org/index.html">Documentation for LDAPjs</a>.</li>
|
||||
<li>Github: <a href="https://github.com/ldapjs/node-ldapjs">ldapjs</a>.</li>
|
||||
<li>Wikipedia: <a href="https://en.wikipedia.org/wiki/LDAP_injection">LDAP injection</a>.</li>
|
||||
<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>
|
||||
<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>
|
||||
</references>
|
||||
</qhelp>
|
||||
@@ -1,20 +0,0 @@
|
||||
/**
|
||||
* @name LDAP query built from user-controlled sources
|
||||
* @description Building an LDAP query from user-controlled sources is vulnerable to insertion of
|
||||
* malicious LDAP code by the user.
|
||||
* @kind path-problem
|
||||
* @problem.severity error
|
||||
* @precision high
|
||||
* @id javascript/ldap-injection
|
||||
* @tags security
|
||||
* external/cwe/cwe-090
|
||||
*/
|
||||
|
||||
import javascript
|
||||
import DataFlow::PathGraph
|
||||
import LdapInjection::LdapInjection
|
||||
|
||||
from LdapInjectionConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink
|
||||
where config.hasFlowPath(source, sink)
|
||||
select sink.getNode(), source, sink, "$@ might include code from $@.",
|
||||
sink.getNode().(Sink).getQueryCall(), "LDAP query call", source.getNode(), "user-provided value"
|
||||
@@ -1,25 +0,0 @@
|
||||
import javascript
|
||||
|
||||
module LdapInjection {
|
||||
import LdapInjectionCustomizations::LdapInjection
|
||||
|
||||
/**
|
||||
* A taint-tracking configuration for reasoning about LDAP injection vulnerabilities.
|
||||
*/
|
||||
class LdapInjectionConfiguration extends TaintTracking::Configuration {
|
||||
LdapInjectionConfiguration() { this = "LdapInjection" }
|
||||
|
||||
override predicate isSource(DataFlow::Node source) { source instanceof Source }
|
||||
|
||||
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
|
||||
|
||||
override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
|
||||
|
||||
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
|
||||
exists(LdapjsParseFilter filter |
|
||||
pred = filter.getArgument(0) and
|
||||
succ = filter
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1,73 +0,0 @@
|
||||
/**
|
||||
* Provides default sources, sinks and sanitizers for reasoning about
|
||||
* LDAP injection vulnerabilities, as well as extension points for
|
||||
* adding your own.
|
||||
*/
|
||||
|
||||
import javascript
|
||||
|
||||
module LdapInjection {
|
||||
import Ldapjs::Ldapjs
|
||||
|
||||
/**
|
||||
* A data flow source for LDAP injection vulnerabilities.
|
||||
*/
|
||||
abstract class Source extends DataFlow::Node { }
|
||||
|
||||
/**
|
||||
* A data flow sink for LDAP injection vulnerabilities.
|
||||
*/
|
||||
abstract class Sink extends DataFlow::Node {
|
||||
/**
|
||||
* Gets the LDAP query call that the sink flows into.
|
||||
*/
|
||||
abstract DataFlow::Node getQueryCall();
|
||||
}
|
||||
|
||||
/**
|
||||
* A sanitizer for LDAP injection vulnerabilities.
|
||||
*/
|
||||
abstract class Sanitizer extends DataFlow::Node { }
|
||||
|
||||
/**
|
||||
* A source of remote user input, considered as a flow source for LDAP injection.
|
||||
*/
|
||||
class RemoteSource extends Source {
|
||||
RemoteSource() { this instanceof RemoteFlowSource }
|
||||
}
|
||||
|
||||
/**
|
||||
* An LDAP filter for an API call that executes an operation against the LDAP server.
|
||||
*/
|
||||
class LdapjsSearchFilterAsSink extends Sink {
|
||||
LdapjsSearchFilterAsSink() { this instanceof LdapjsSearchFilter }
|
||||
|
||||
override DataFlow::InvokeNode getQueryCall() {
|
||||
result = this.(LdapjsSearchFilter).getQueryCall()
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* An LDAP DN argument for an API call that executes an operation against the LDAP server.
|
||||
*/
|
||||
class LdapjsDNArgumentAsSink extends Sink {
|
||||
LdapjsDNArgumentAsSink() { this instanceof LdapjsDNArgument }
|
||||
|
||||
override DataFlow::InvokeNode getQueryCall() { result = this.(LdapjsDNArgument).getQueryCall() }
|
||||
}
|
||||
|
||||
/**
|
||||
* A call to a function whose name suggests that it escapes LDAP search query parameter.
|
||||
*/
|
||||
class FilterOrDNSanitizationCall extends Sanitizer, DataFlow::CallNode {
|
||||
FilterOrDNSanitizationCall() {
|
||||
exists(string sanitize, string input |
|
||||
sanitize = "(?:escape|saniti[sz]e|validate|filter)" and
|
||||
input = "[Ii]nput?"
|
||||
|
|
||||
this.getCalleeName()
|
||||
.regexpMatch("(?i)(" + sanitize + input + ")" + "|(" + input + sanitize + ")")
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user