hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-05-01 03:35:13 +02:00
Code Issues Packages Projects Releases Wiki Activity

model ldap filters as taint steps

Browse Source
This commit is contained in:
Erik Krogh Kristensen
2021-09-30 13:32:55 +02:00
parent 9b5ff66b68
commit c55b7bcd85
5 changed files with 104 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files

97
javascript/ql/lib/semmle/javascript/frameworks/Ldapjs.qll
View File

@@ -5,38 +5,38 @@
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 API::Node { }
/**
* Gets a data flow source node for the ldapjs library.
*/
private API::Node ldapjs() { result = API::moduleImport("ldapjs") }
private API::Node ldapjs() { result = API::moduleImport("ldapjs") } // TODO: createServer, parseDN.
/**
* Gets a data flow source node for the ldapjs client.
* Gets an LDAP client.
*/
class LdapjsClient extends LdapClient {
LdapjsClient() { this = ldapjs().getMember("createClient").getReturn() }
private API::Node ldapClient() { result = ldapjs().getMember("createClient").getReturn() }
/**
* A call to the ldapjs Client API methods.
*/
class LdapjsClientAPICall extends API::CallNode {
string methodName;
LdapjsClientAPICall() {
methodName = ["add", "bind", "compare", "del", "modify", "modifyDN", "search"] and
this = ldapClient().getMember(methodName).getACall()
}
string getMethodName() { result = methodName }
}
/**
* Gets a data flow node for the client `search` options.
*/
class LdapjsSearchOptions extends API::Node {
API::CallNode queryCall;
LdapjsClientAPICall queryCall;
LdapjsSearchOptions() {
queryCall = any(LdapjsClient client).getMember("search").getACall() and
queryCall.getMethodName() = "search" and
this = queryCall.getParameter(1)
}
@@ -46,29 +46,6 @@ module Ldapjs {
API::CallNode getQueryCall() { result = queryCall }
}
/**
* A filter used in a `search` operation against the LDAP server.
*/
class LdapjsSearchFilter extends DataFlow::Node {
LdapjsSearchOptions options;
LdapjsSearchFilter() { this = options.getMember("filter").getARhs() }
/**
* Gets the LDAP query call that this filter is used in.
*/
API::CallNode getQueryCall() { result = options.getQueryCall() }
}
/**
* A call to the ldapjs Client API methods.
*/
class LdapjsClientAPICall extends API::CallNode {
LdapjsClientAPICall() {
this = any(LdapjsClient client).getMember(getLdapjsClientDNMethodName()).getACall()
}
}
/**
* A distinguished name (DN) used in a Client API call against the LDAP server.
*/
@@ -83,10 +60,46 @@ module Ldapjs {
API::CallNode getQueryCall() { result = queryCall }
}
/**
* A creation of an Ldap filter that doesn't sanitizes the input.
*/
abstract class LdapFilter extends DataFlow::Node {
/**
* The input that creates (part of) an Ldap filter.
*/
abstract DataFlow::Node getInput();
/**
* The resulting Ldap filter.
*/
abstract DataFlow::Node getOutput();
}
/**
* Ldapjs parseFilter method call.
*/
class LdapjsParseFilter extends DataFlow::CallNode {
private class LdapjsParseFilter extends LdapFilter, API::CallNode {
LdapjsParseFilter() { this = ldapjs().getMember("parseFilter").getACall() }
override DataFlow::Node getInput() { result = this.getArgument(0) }
override DataFlow::Node getOutput() { result = this }
}
/**
* A filter used in call to "search" on an LDAP client.
* We model that as a step from the ".filter" write to the options object itself.
*/
class LdapjsSearchFilter extends LdapFilter {
LdapjsSearchOptions options;
LdapjsSearchFilter() {
options = ldapClient().getMember("search").getACall().getParameter(1) and
this = options.getARhs()
}
override DataFlow::Node getInput() { result = options.getMember("filter").getARhs() }
override DataFlow::Node getOutput() { result = this }
}
}

27
javascript/ql/lib/semmle/javascript/security/dataflow/SqlInjectionCustomizations.qll
View File

@@ -43,27 +43,14 @@ module SqlInjection {
}
/**
* An LDAP filter for an API call that executes an operation against the LDAP server.
* An LDAPjs sink.
*/
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 }
class LdapJSSink extends Sink {
LdapJSSink() {
this instanceof Ldapjs::LdapjsDNArgument
or
this = any(Ldapjs::LdapjsSearchOptions opt).getARhs()
}
}
/**

7
javascript/ql/lib/semmle/javascript/security/dataflow/SqlInjectionQuery.qll
View File

@@ -24,4 +24,11 @@ class Configuration extends TaintTracking::Configuration {
super.isSanitizer(node) or
node instanceof Sanitizer
}
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
exists(Ldapjs::LdapFilter filter |
pred = filter.getInput() and
succ = filter.getOutput()
)
}
}

36
javascript/ql/test/query-tests/Security/CWE-089/untyped/SqlInjection.expected
View File

@@ -77,13 +77,23 @@ nodes
| ldap.js:22:18:22:24 | q.query |
| ldap.js:22:18:22:33 | q.query.username |
| ldap.js:25:13:25:57 | `(\|(nam ... ame}))` |
| ldap.js:25:13:25:57 | `(\|(nam ... ame}))` |
| ldap.js:25:24:25:31 | username |
| ldap.js:25:46:25:53 | username |
| ldap.js:32:15:32:59 | `(\|(nam ... ame}))` |
| ldap.js:28:30:28:34 | opts1 |
| ldap.js:28:30:28:34 | opts1 |
| ldap.js:32:5:32:61 | { filte ... e}))` } |
| ldap.js:32:5:32:61 | { filte ... e}))` } |
| ldap.js:32:15:32:59 | `(\|(nam ... ame}))` |
| ldap.js:32:26:32:33 | username |
| ldap.js:32:48:32:55 | username |
| ldap.js:63:9:65:3 | parsedFilter |
| ldap.js:63:24:65:3 | ldap.pa ... ))`\\n ) |
| ldap.js:64:5:64:49 | `(\|(nam ... ame}))` |
| ldap.js:64:16:64:23 | username |
| ldap.js:64:38:64:45 | username |
| ldap.js:66:30:66:53 | { filte ... ilter } |
| ldap.js:66:30:66:53 | { filte ... ilter } |
| ldap.js:66:40:66:51 | parsedFilter |
| marsdb-flow-to.js:10:9:10:18 | query |
| marsdb-flow-to.js:10:17:10:18 | {} |
| marsdb-flow-to.js:11:17:11:24 | req.body |
@@ -468,17 +478,26 @@ edges
| ldap.js:22:7:22:33 | username | ldap.js:25:46:25:53 | username |
| ldap.js:22:7:22:33 | username | ldap.js:32:26:32:33 | username |
| ldap.js:22:7:22:33 | username | ldap.js:32:48:32:55 | username |
| ldap.js:22:7:22:33 | username | ldap.js:64:16:64:23 | username |
| ldap.js:22:7:22:33 | username | ldap.js:64:38:64:45 | username |
| ldap.js:22:18:22:18 | q | ldap.js:22:18:22:24 | q.query |
| ldap.js:22:18:22:24 | q.query | ldap.js:22:18:22:33 | q.query.username |
| ldap.js:22:18:22:33 | q.query.username | ldap.js:22:7:22:33 | username |
| ldap.js:25:24:25:31 | username | ldap.js:25:13:25:57 | `(\|(nam ... ame}))` |
| ldap.js:25:13:25:57 | `(\|(nam ... ame}))` | ldap.js:28:30:28:34 | opts1 |
| ldap.js:25:13:25:57 | `(\|(nam ... ame}))` | ldap.js:28:30:28:34 | opts1 |
| ldap.js:25:24:25:31 | username | ldap.js:25:13:25:57 | `(\|(nam ... ame}))` |
| ldap.js:25:46:25:53 | username | ldap.js:25:13:25:57 | `(\|(nam ... ame}))` |
| ldap.js:25:46:25:53 | username | ldap.js:25:13:25:57 | `(\|(nam ... ame}))` |
| ldap.js:32:26:32:33 | username | ldap.js:32:15:32:59 | `(\|(nam ... ame}))` |
| ldap.js:32:15:32:59 | `(\|(nam ... ame}))` | ldap.js:32:5:32:61 | { filte ... e}))` } |
| ldap.js:32:15:32:59 | `(\|(nam ... ame}))` | ldap.js:32:5:32:61 | { filte ... e}))` } |
| ldap.js:32:26:32:33 | username | ldap.js:32:15:32:59 | `(\|(nam ... ame}))` |
| ldap.js:32:48:32:55 | username | ldap.js:32:15:32:59 | `(\|(nam ... ame}))` |
| ldap.js:32:48:32:55 | username | ldap.js:32:15:32:59 | `(\|(nam ... ame}))` |
| ldap.js:63:9:65:3 | parsedFilter | ldap.js:66:40:66:51 | parsedFilter |
| ldap.js:63:24:65:3 | ldap.pa ... ))`\\n ) | ldap.js:63:9:65:3 | parsedFilter |
| ldap.js:64:5:64:49 | `(\|(nam ... ame}))` | ldap.js:63:24:65:3 | ldap.pa ... ))`\\n ) |
| ldap.js:64:16:64:23 | username | ldap.js:64:5:64:49 | `(\|(nam ... ame}))` |
| ldap.js:64:38:64:45 | username | ldap.js:64:5:64:49 | `(\|(nam ... ame}))` |
| ldap.js:66:40:66:51 | parsedFilter | ldap.js:66:30:66:53 | { filte ... ilter } |
| ldap.js:66:40:66:51 | parsedFilter | ldap.js:66:30:66:53 | { filte ... ilter } |
| marsdb-flow-to.js:10:9:10:18 | query | marsdb-flow-to.js:14:17:14:21 | query |
| marsdb-flow-to.js:10:9:10:18 | query | marsdb-flow-to.js:14:17:14:21 | query |
| marsdb-flow-to.js:10:17:10:18 | {} | marsdb-flow-to.js:10:9:10:18 | query |
@@ -887,8 +906,9 @@ edges
| json-schema-validator.js:55:22:55:26 | query | json-schema-validator.js:50:34:50:47 | req.query.data | json-schema-validator.js:55:22:55:26 | query | This query depends on $@. | json-schema-validator.js:50:34:50:47 | req.query.data | a user-provided value |
| json-schema-validator.js:59:22:59:26 | query | json-schema-validator.js:50:34:50:47 | req.query.data | json-schema-validator.js:59:22:59:26 | query | This query depends on $@. | json-schema-validator.js:50:34:50:47 | req.query.data | a user-provided value |
| json-schema-validator.js:61:22:61:26 | query | json-schema-validator.js:50:34:50:47 | req.query.data | json-schema-validator.js:61:22:61:26 | query | This query depends on $@. | json-schema-validator.js:50:34:50:47 | req.query.data | a user-provided value |
| ldap.js:25:13:25:57 | `(\|(nam ... ame}))` | ldap.js:20:21:20:27 | req.url | ldap.js:25:13:25:57 | `(\|(nam ... ame}))` | This query depends on $@. | ldap.js:20:21:20:27 | req.url | a user-provided value |
| ldap.js:32:15:32:59 | `(\|(nam ... ame}))` | ldap.js:20:21:20:27 | req.url | ldap.js:32:15:32:59 | `(\|(nam ... ame}))` | This query depends on $@. | ldap.js:20:21:20:27 | req.url | a user-provided value |
| ldap.js:28:30:28:34 | opts1 | ldap.js:20:21:20:27 | req.url | ldap.js:28:30:28:34 | opts1 | This query depends on $@. | ldap.js:20:21:20:27 | req.url | a user-provided value |
| ldap.js:32:5:32:61 | { filte ... e}))` } | ldap.js:20:21:20:27 | req.url | ldap.js:32:5:32:61 | { filte ... e}))` } | This query depends on $@. | ldap.js:20:21:20:27 | req.url | a user-provided value |
| ldap.js:66:30:66:53 | { filte ... ilter } | ldap.js:20:21:20:27 | req.url | ldap.js:66:30:66:53 | { filte ... ilter } | This query depends on $@. | ldap.js:20:21:20:27 | req.url | a user-provided value |
| marsdb-flow-to.js:14:17:14:21 | query | marsdb-flow-to.js:11:17:11:24 | req.body | marsdb-flow-to.js:14:17:14:21 | query | This query depends on $@. | marsdb-flow-to.js:11:17:11:24 | req.body | a user-provided value |
| marsdb.js:16:12:16:16 | query | marsdb.js:13:17:13:24 | req.body | marsdb.js:16:12:16:16 | query | This query depends on $@. | marsdb.js:13:17:13:24 | req.body | a user-provided value |
| minimongo.js:18:12:18:16 | query | minimongo.js:15:17:15:24 | req.body | minimongo.js:18:12:18:16 | query | This query depends on $@. | minimongo.js:15:17:15:24 | req.body | a user-provided value |

9
javascript/ql/test/query-tests/Security/CWE-089/untyped/ldap.js
View File

@@ -22,10 +22,10 @@ const server = http.createServer((req, res) => {
let username = q.query.username;
var opts1 = {
filter: `(|(name=${username})(username=${username}))`, // NOT OK
filter: `(|(name=${username})(username=${username}))`,
};
client.search("o=example", opts1, function (err, res) {});
client.search("o=example", opts1, function (err, res) {}); // NOT OK
client.search(
"o=example",
@@ -59,6 +59,11 @@ const server = http.createServer((req, res) => {
});
client.search("o=example", { filter: f }, function (err, res) {});
const parsedFilter = ldap.parseFilter(
`(|(name=${username})(username=${username}))`
);
client.search("o=example", { filter: parsedFilter }, function (err, res) {}); // NOT OK
});
server.listen(389, () => {});