Skip to content

Commit 6a9277b

Browse files
committed
recognize string sanitizers for ldap-injection
1 parent 51b56a9 commit 6a9277b

File tree

2 files changed

+10
-11
lines changed

2 files changed

+10
-11
lines changed

javascript/ql/lib/semmle/javascript/security/dataflow/SqlInjectionCustomizations.qll

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -58,18 +58,17 @@ module SqlInjection {
5858
}
5959
}
6060

61+
import semmle.javascript.security.IncompleteBlacklistSanitizer as IncompleteBlacklistSanitizer
62+
6163
/**
62-
* A call to a function whose name suggests that it escapes LDAP search query parameter.
64+
* A chain of replace calls that replaces all unsafe chars for ldap injection.
65+
* For simplicity it's used as a sanitizer for all of `js/sql-injection`.
6366
*/
64-
class FilterOrDNSanitizationCall extends Sanitizer, DataFlow::CallNode {
65-
// TODO: remove, or use something else? (AdhocWhitelistSanitizer?)
66-
FilterOrDNSanitizationCall() {
67-
exists(string sanitize, string input |
68-
sanitize = "(?:escape|saniti[sz]e|validate|filter)" and
69-
input = "[Ii]nput?"
70-
|
71-
this.getCalleeName()
72-
.regexpMatch("(?i)(" + sanitize + input + ")" + "|(" + input + sanitize + ")")
67+
class LDAPStringSanitizer extends Sanitizer,
68+
IncompleteBlacklistSanitizer::StringReplaceCallSequence {
69+
LDAPStringSanitizer() {
70+
forall(string char | char = ["*", "(", ")", "\\", "/"] |
71+
this.getAMember().getAReplacedString() = char
7372
)
7473
}
7574
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ const server = http.createServer((req, res) => {
3636
// GOOD
3737
client.search(
3838
"o=example",
39-
{
39+
{ // OK
4040
filter: `(|(name=${sanitizeInput(username)})(username=${sanitizeInput(
4141
username
4242
)}))`,

0 commit comments

Comments
 (0)