Apply suggestions from code review

Co-authored-by: Rasmus Wriedt Larsen <rasmuswriedtlarsen@gmail.com>
This commit is contained in:
Jorge
2021-09-07 18:37:33 +02:00
committed by GitHub
parent 64b305cf7a
commit 1bc16fb31e
4 changed files with 8 additions and 5 deletions

View File

@@ -17,7 +17,7 @@ to be sent in cleartext making it easier for an attacker to intercept it.</p>
<p>The first one sets <code>use_SSL</code> to true as a keyword argument whereas the second one fails to provide a value for it, so
the default one is used (<code>False</code>).</p>
<sample src="LDAPInsecureAuth.py" />
<sample src="examples/LDAPInsecureAuth.py" />
</example>
</qhelp>

View File

@@ -3,7 +3,7 @@
* @description Python LDAP Insecure LDAP Authentication
* @kind path-problem
* @problem.severity error
* @id python/insecure-ldap-auth
* @id py/insecure-ldap-auth
* @tags experimental
* security
* external/cwe/cwe-522

View File

@@ -115,9 +115,10 @@ private module LDAP {
initialize = ldapInitialize().getACall() and
(
// ldap_connection.start_tls_s()
// see https://www.python-ldap.org/en/python-ldap-3.3.0/reference/ldap.html#ldap.LDAPObject.start_tls_s
exists(DataFlow::AttrRead startTLS |
startTLS.getObject().getALocalSource() = initialize and
startTLS.getAttributeName().matches("%start_tls%")
startTLS.getAttributeName() = "start_tls_s"
)
or
// ldap_connection.set_option(ldap.OPT_X_TLS_%s, True)

View File

@@ -23,7 +23,8 @@ class LDAPFullHost extends StrConst {
exists(string s |
s = this.getText() and
s.regexpMatch(getFullHostRegex()) and
not s.substring(7, s.length()).regexpMatch(getPrivateHostRegex()) // No need to check for ldaps, it would be SSL by default.
// check what comes after the `ldap://` prefix
not s.substring(7, s.length()).regexpMatch(getPrivateHostRegex())
)
}
}
@@ -36,7 +37,7 @@ class LDAPPrivateHost extends StrConst {
LDAPPrivateHost() { this.getText().regexpMatch(getPrivateHostRegex()) }
}
predicate concatAndCompareAgainstFullHostRegex(Expr schema, Expr host) {
predicate concatAndCompareAgainstFullHostRegex(LDAPSchema schema, StrConst host) {
schema instanceof LDAPSchema and
not host instanceof LDAPPrivateHost and
exists(string full_host |
@@ -96,6 +97,7 @@ class LDAPInsecureAuthConfig extends TaintTracking::Configuration {
override predicate isSource(DataFlow::Node source) {
source instanceof RemoteFlowSource or
source.asExpr() instanceof LDAPFullHost or
source.asExpr() instanceof LDAPBothStrings or
source.asExpr() instanceof LDAPBothVar or
source.asExpr() instanceof LDAPVarString or