hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-28 10:15:14 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #6781 from erik-krogh/ldap

Browse Source 
JS: Move LDAP injection out of experimental
This commit is contained in:
Erik Krogh Kristensen
2021-11-02 13:35:32 +01:00
committed by GitHub
parent 7d0152f3c0 7a96b8e9e1
commit 54fba2d6a1
17 changed files with 251 additions and 353 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
javascript/ql/src/Security/CWE-089/SqlInjection.ql
View File

@@ -9,6 +9,8 @@
* @id js/sql-injection
* @tags security
* external/cwe/cwe-089
* external/cwe/cwe-090
* external/cwe/cwe-943
*/
import javascript

50
javascript/ql/src/experimental/Security/CWE-090/LdapInjection.qhelp
View File

@@ -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>

20
javascript/ql/src/experimental/Security/CWE-090/LdapInjection.ql
View File

@@ -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"

25
javascript/ql/src/experimental/Security/CWE-090/LdapInjection.qll
View File

@@ -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
)
}
}
}

69
javascript/ql/src/experimental/Security/CWE-090/LdapInjectionCustomizations.qll
View File

@@ -1,69 +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 instanceof LdapjsSearchFilter {
override DataFlow::InvokeNode getQueryCall() {
result = LdapjsSearchFilter.super.getQueryCall()
}
}
/**
* An LDAP DN argument for an API call that executes an operation against the LDAP server.
*/
class LdapjsDNArgumentAsSink extends Sink instanceof LdapjsDNArgument {
override DataFlow::InvokeNode getQueryCall() { result = LdapjsDNArgument.super.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 + ")")
)
}
}
}

92
javascript/ql/src/experimental/Security/CWE-090/Ldapjs.qll
View File

@@ -1,92 +0,0 @@
/**
* Provides classes for working with [ldapjs](https://github.com/ldapjs/node-ldapjs) (Client only)
*/
import javascript
module Ldapjs {
/**
* Gets a method name on an LDAPjs client that accepts a DN as the first argument.
*/
private string getLdapjsClientDNMethodName() {
result = ["add", "bind", "compare", "del", "modify", "modifyDN", "search"]
}
/**
* Gets a data flow source node for an LDAP client.
*/
abstract class LdapClient extends DataFlow::SourceNode { }
/**
* Gets a data flow source node for the ldapjs library.
*/
private DataFlow::SourceNode ldapjs() { result = DataFlow::moduleImport("ldapjs") }
/**
* Gets a data flow source node for the ldapjs client.
*/
class LdapjsClient extends LdapClient {
LdapjsClient() { this = ldapjs().getAMemberCall("createClient") }
}
/**
* Gets a data flow node for the client `search` options.
*/
class LdapjsSearchOptions extends DataFlow::SourceNode {
DataFlow::CallNode queryCall;
LdapjsSearchOptions() {
queryCall = any(LdapjsClient client).getAMemberCall("search") and
this = queryCall.getArgument(1).getALocalSource()
}
/**
* Gets the LDAP query call that these options are used in.
*/
DataFlow::InvokeNode getQueryCall() { result = queryCall }
}
/**
* A filter used in a `search` operation against the LDAP server.
*/
class LdapjsSearchFilter extends DataFlow::Node {
LdapjsSearchOptions options;
LdapjsSearchFilter() { this = options.getAPropertyWrite("filter").getRhs() }
/**
* Gets the LDAP query call that this filter is used in.
*/
DataFlow::InvokeNode getQueryCall() { result = options.getQueryCall() }
}
/**
* A call to the ldapjs Client API methods.
*/
class LdapjsClientAPICall extends DataFlow::CallNode {
LdapjsClientAPICall() {
this = any(LdapjsClient client).getAMemberCall(getLdapjsClientDNMethodName())
}
}
/**
* A distinguished name (DN) used in a Client API call against the LDAP server.
*/
class LdapjsDNArgument extends DataFlow::Node {
LdapjsClientAPICall queryCall;
LdapjsDNArgument() { this = queryCall.getArgument(0) }
/**
* Gets the LDAP query call that this DN is used in.
*/
DataFlow::InvokeNode getQueryCall() { result = queryCall }
}
/**
* Ldapjs parseFilter method call.
*/
class LdapjsParseFilter extends DataFlow::CallNode {
LdapjsParseFilter() { this = ldapjs().getAMemberCall("parseFilter") }
}
}

21
javascript/ql/src/experimental/Security/CWE-090/examples/example_bad1.js
View File

@@ -1,21 +0,0 @@
const http = require('http');
const url = require('url');
const ldap = require('ldapjs');
const client = ldap.createClient({
url: 'ldap://127.0.0.1:1389'
});
const server = http.createServer((req, res) => {
let q = url.parse(req.url, true);
let username = q.query.username;
var opts = {
// BAD
filter: `(|(name=${username})(username=${username}))`
};
client.search('o=example', opts, function (err, res) {
});
});

16
javascript/ql/src/experimental/Security/CWE-090/examples/example_bad2.js
View File

@@ -1,16 +0,0 @@
const http = require('http');
const url = require('url');
const ldap = require('ldapjs');
const client = ldap.createClient({
url: 'ldap://127.0.0.1:1389'
});
const server = http.createServer((req, res) => {
let q = url.parse(req.url, true);
let username = q.query.username;
// BAD
client.search('o=example', { filter: `(|(name=${username})(username=${username}))` }, function (err, res) {
});
});

31
javascript/ql/src/experimental/Security/CWE-090/examples/example_good1.js
View File

@@ -1,31 +0,0 @@
const http = require('http');
const url = require('url');
const ldap = require('ldapjs');
const client = ldap.createClient({
url: 'ldap://127.0.0.1:1389'
});
// https://github.com/vesse/node-ldapauth-fork/commit/3feea43e243698bcaeffa904a7324f4d96df60e4
const sanitizeInput = function (input) {
return input
.replace(/\*/g, '\\2a')
.replace(/\(/g, '\\28')
.replace(/\)/g, '\\29')
.replace(/\\/g, '\\5c')
.replace(/\0/g, '\\00')
.replace(/\//g, '\\2f');
};
const server = http.createServer((req, res) => {
let q = url.parse(req.url, true);
let username = q.query.username;
// GOOD
username = sanitizeInput(username);
client.search('o=example', { filter: `(|(name=${username})(username=${username}))` }, function (err, res) {
});
});

29
javascript/ql/src/experimental/Security/CWE-090/examples/example_good2.js
View File

@@ -1,29 +0,0 @@
const http = require('http');
const url = require('url');
const ldap = require('ldapjs');
const client = ldap.createClient({
url: 'ldap://127.0.0.1:1389'
});
const server = http.createServer((req, res) => {
let q = url.parse(req.url, true);
let username = q.query.username;
// GOOD (https://github.com/ldapjs/node-ldapjs/issues/181)
let f = new OrFilter({
filters: [
new EqualityFilter({
attribute: 'name',
value: username
}),
new EqualityFilter({
attribute: 'username',
value: username
})
]
});
client.search('o=example', { filter: f }, function (err, res) {
});
});