Merge pull request #4118 from dellalibera/js/ldap

[javascript] CodeQL to detect LDAP Injection

Author: erik-krogh

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

@@ -0,0 +1,50 @@
+<!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>

javascript/ql/src/experimental/Security/CWE-090/LdapInjection.ql

@@ -0,0 +1,20 @@
+/**
+ * @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"

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

@@ -0,0 +1,18 @@
+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 }
+  }
+}

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

@@ -0,0 +1,85 @@
+/**
+ * Provides default sources, sinks and sanitizers for reasoning about
+ * LDAP injection vulnerabilities, as well as extension points for
+ * adding your own.
+ */
+
+import javascript
+import Ldapjs::Ldapjs
+
+module LdapInjection {
+  /**
+   * 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 + ")")
+      )
+    }
+  }
+
+  /**
+   * A step through the parseFilter API (https://github.com/ldapjs/node-ldapjs/issues/181).
+   */
+  class StepThroughParseFilter extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode {
+    StepThroughParseFilter() { this instanceof LdapjsParseFilter }
+
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      pred = this.getArgument(0) and
+      succ = this
+    }
+  }
+}

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

@@ -0,0 +1,92 @@
+/**
+ * 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") }
+  }
+}

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

@@ -0,0 +1,21 @@
+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) {
+
+    });
+});

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

@@ -0,0 +1,16 @@
+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) {
+    });
+});

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

@@ -0,0 +1,31 @@
+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) {
+    });
+
+});

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

@@ -0,0 +1,29 @@
+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) {
+    });
+});