Apply suggestions from code review

Co-authored-by: Rasmus Wriedt Larsen <[email protected]>

Author: jorgectf

python/ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.qhelp

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

python/ql/src/experimental/Security/CWE-522/LDAPInsecureAuth.ql

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

python/ql/src/experimental/semmle/python/frameworks/LDAP.qll

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

python/ql/src/experimental/semmle/python/security/LDAPInsecureAuth.qll

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