Suggested Changes

Author: maikypedia

ruby/ql/lib/codeql/ruby/frameworks/Ldap.qll

@@ -14,8 +14,8 @@ module NetLdap {
   /**
    * Flow summary for `Net::LDAP.new`. This method establishes a connection to a LDAP server.
    */
-  private class LdapSummary extends SummarizedCallable {
-    LdapSummary() { this = "Net::LDAP.new" }
+  private class LdapConnSummary extends SummarizedCallable {
+    LdapConnSummary() { this = "Net::LDAP.new" }
 
     override MethodCall getACall() { result = any(NetLdapConnection l).asExpr().getExpr() }
 
@@ -24,6 +24,19 @@ module NetLdap {
     }
   }
 
+  /**
+   * Flow summary for `Net::LDAP.Filter`.
+   */
+  private class LdapFilterSummary extends SummarizedCallable {
+    LdapFilterSummary() { this = "Net::LDAP::Filter" }
+
+    override MethodCall getACall() { result = any(NetLdapFilter l).asExpr().getExpr() }
+
+    override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
+      input = ["Argument[0]", "Argument[1]"] and output = "ReturnValue" and preservesValue = false
+    }
+  }
+
   /** Net LDAP Api Node */
   private API::Node ldap() { result = API::getTopLevelMember("Net").getMember("LDAP") }
 

ruby/ql/lib/codeql/ruby/security/LdapInjectionCustomizations.qll

@@ -27,23 +27,19 @@ module LdapInjection {
    * Additional taint steps for "LDAP Injection" vulnerabilities.
    */
   predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
-    filterTaintStep(nodeFrom, nodeTo)
+    attributeArrayTaintStep(nodeFrom, nodeTo)
   }
 
-  private predicate filterTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
+  /**
+   * Additional taint step to handle elements inside an array,
+   * specifically in the context of the following LDAP search function:
+   *
+   * ldap.search(base: "", filter: "", attributes: [name])
+   */
+  private predicate attributeArrayTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
     exists(DataFlow::CallNode filterCall |
-      (
-        filterCall =
-          API::getTopLevelMember("Net")
-              .getMember("LDAP")
-              .getMember("Filter")
-              .getAMethodCall([
-                  "begins", "bineq", "contains", "ends", "eq", "equals", "ex", "ge", "le", "ne",
-                  "present"
-                ]) or
-        filterCall.getMethodName() = "[]"
-      ) and
-      n1 = filterCall.getArgument([0, 1]) and
+      filterCall.getMethodName() = "[]" and
+      n1 = filterCall.getArgument(_) and
       n2 = filterCall
     )
   }
@@ -72,4 +68,14 @@ module LdapInjection {
   private class StringConstArrayInclusionCallAsSanitizer extends Sanitizer,
     StringConstArrayInclusionCallBarrier
   { }
+
+  /**
+   * A call to `Net::LDAP::Filter.escape`, considered as a sanitizer.
+   */
+  class NetLdapFilterEscapeSanitization extends Sanitizer {
+    NetLdapFilterEscapeSanitization() {
+      this =
+        API::getTopLevelMember("Net").getMember("LDAP").getMember("Filter").getAMethodCall("escape")
+    }
+  }
 }

ruby/ql/test/query-tests/experimental/LdapInjection/Ldapinjection.expected

@@ -4,10 +4,12 @@ edges
 | LdapInjection.rb:5:10:5:20 | ...[...] | LdapInjection.rb:5:5:5:6 | dc |
 | LdapInjection.rb:9:5:9:8 | name | LdapInjection.rb:29:62:29:73 | "cn=#{...}" |
 | LdapInjection.rb:9:5:9:8 | name | LdapInjection.rb:33:87:33:92 | call to [] |
-| LdapInjection.rb:9:5:9:8 | name | LdapInjection.rb:37:5:37:10 | filter |
+| LdapInjection.rb:9:5:9:8 | name | LdapInjection.rb:37:41:37:44 | name |
 | LdapInjection.rb:9:12:9:17 | call to params | LdapInjection.rb:9:12:9:29 | ...[...] |
 | LdapInjection.rb:9:12:9:29 | ...[...] | LdapInjection.rb:9:5:9:8 | name |
 | LdapInjection.rb:37:5:37:10 | filter | LdapInjection.rb:38:62:38:67 | filter |
+| LdapInjection.rb:37:14:37:45 | call to eq | LdapInjection.rb:37:5:37:10 | filter |
+| LdapInjection.rb:37:41:37:44 | name | LdapInjection.rb:37:14:37:45 | call to eq |
 nodes
 | LdapInjection.rb:5:5:5:6 | dc | semmle.label | dc |
 | LdapInjection.rb:5:10:5:15 | call to params | semmle.label | call to params |
@@ -19,6 +21,8 @@ nodes
 | LdapInjection.rb:29:62:29:73 | "cn=#{...}" | semmle.label | "cn=#{...}" |
 | LdapInjection.rb:33:87:33:92 | call to [] | semmle.label | call to [] |
 | LdapInjection.rb:37:5:37:10 | filter | semmle.label | filter |
+| LdapInjection.rb:37:14:37:45 | call to eq | semmle.label | call to eq |
+| LdapInjection.rb:37:41:37:44 | name | semmle.label | name |
 | LdapInjection.rb:38:62:38:67 | filter | semmle.label | filter |
 subpaths
 #select