add filterTaintStep, qhelp file and test files

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

@@ -8,14 +8,6 @@ private import codeql.ruby.dataflow.FlowSummary
 private import codeql.ruby.Concepts
 private import codeql.ruby.CFG
 private import codeql.ruby.AST
-private import codeql.ruby.Concepts
-private import codeql.ruby.controlflow.CfgNodes
-private import codeql.ruby.DataFlow
-private import codeql.ruby.dataflow.internal.DataFlowDispatch
-private import codeql.ruby.dataflow.internal.DataFlowPrivate
-private import codeql.ruby.ApiGraphs
-private import codeql.ruby.frameworks.Stdlib
-private import codeql.ruby.frameworks.Core
 
 /**
  * Provides modeling for `net-ldap` a ruby library for LDAP.
@@ -27,53 +19,32 @@ module NetLdap {
   private class LdapSummary extends SummarizedCallable {
     LdapSummary() { this = "Net::LDAP.new" }
 
-    override MethodCall getACall() { result = any(LdapConnection l).asExpr().getExpr() }
+    override MethodCall getACall() { result = any(NetLdapConnection l).asExpr().getExpr() }
 
     override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
       input = "Argument[0]" and output = "ReturnValue" and preservesValue = false
     }
   }
 
-  private class LdapConnection extends DataFlow::CallNode {
-    LdapConnection() {
-      this = API::getTopLevelMember("Net").getMember("LDAP").getAnInstantiation() or
-      this = API::getTopLevelMember("Net").getMember("LDAP").getAMethodCall(["open"])
-    }
+  /** Net LDAP Api Nodde */
+  private API::Node ldap() { result = API::getTopLevelMember("Net").getMember("LDAP") }
+
+  /** A call that establishes a LDAP Connection */
+  private class NetLdapConnection extends DataFlow::CallNode {
+    NetLdapConnection() { this in [ldap().getAnInstantiation(), ldap().getAMethodCall(["open"])] }
   }
 
-  /** A call to ` Net::LDAP::Filter.eq`, considered as a LDAP construction. */
-  private class NetLdapConstruction extends LdapConstruction::Range, DataFlow::CallNode {
-    DataFlow::Node query;
+  /** A call that constructs a LDAP query */
+  private class NetLdapFilter extends LdapConstruction::Range, DataFlow::CallNode {
+    NetLdapFilter() { this = any(ldap().getMember("Filter").getAMethodCall(["eq"])) }
 
-    NetLdapConstruction() {
-      this =
-        API::getTopLevelMember("Net").getMember("LDAP").getMember("Filter").getAMethodCall(["eq"]) and
-      query = this.getArgument([0, 1])
-    }
-
-    override DataFlow::Node getQuery() { result = query }
+    override DataFlow::Node getQuery() { result = this.getArgument([0, 1]) }
   }
 
   /** A call considered as a LDAP execution. */
   private class NetLdapExecution extends LdapExecution::Range, DataFlow::CallNode {
-    DataFlow::Node query;
+    NetLdapExecution() { this = any(NetLdapConnection l).getAMethodCall("search") }
 
-    NetLdapExecution() {
-      // detecta cuando la query es una string pej
-      // ldap.search(base: "ou=#{name},dc=example,dc=com"
-      exists(LdapConnection ldapConnection |
-        this = ldapConnection.getAMethodCall("search") and
-        query = this.getKeywordArgument(_)
-      )
-      or
-      // ignora esta parte
-      exists(LdapConnection ldapConnection, NetLdapConstruction ldapConstruction |
-        this = ldapConnection.getAMethodCall("search") and
-        ldapConstruction = this.getKeywordArgument(_) and
-        query = ldapConstruction
-      )
-    }
-
-    override DataFlow::Node getQuery() { result = query }
+    override DataFlow::Node getQuery() { result = this.getKeywordArgument(_) }
   }
 }

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

@@ -8,8 +8,6 @@ private import codeql.ruby.DataFlow
 private import codeql.ruby.dataflow.BarrierGuards
 private import codeql.ruby.dataflow.RemoteFlowSources
 private import codeql.ruby.ApiGraphs
-private import codeql.ruby.dataflow.FlowSummary
-private import codeql.ruby.CFG
 
 /**
  * Provides default sources, sinks and sanitizers for detecting
@@ -29,12 +27,18 @@ module LdapInjection {
    * Additional taint steps for "LDAP Injection" vulnerabilities.
    */
   predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
-    exists(API::Node n, API::Node n2 |
-      n = API::getTopLevelMember("Net").getMember("LDAP").getMember("Filter")
-    |
-      n2 = API::getTopLevelMember("Net").getMember("LDAP") and
-      nodeTo = n2.getAMethodCall(["new"]).getAMethodCall(["search"]) and
-      nodeFrom = n.getAMethodCall(["eq"]).getArgument([0, 1])
+    filterTaintStep(nodeFrom, nodeTo)
+  }
+
+  private predicate filterTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
+    exists(DataFlow::CallNode filterCall |
+      (
+        filterCall =
+          API::getTopLevelMember("Net").getMember("LDAP").getMember("Filter").getAMethodCall(["eq"]) or
+        filterCall.getMethodName() = ["[]"]
+      ) and
+      n1 = filterCall.getArgument([0, 1]) and
+      n2 = filterCall
     )
   }
 

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

@@ -5,8 +5,8 @@
 
 private import codeql.ruby.DataFlow
 private import codeql.ruby.TaintTracking
-import LdapInjectionCustomizations
-import LdapInjectionCustomizations::LdapInjection
+private import LdapInjectionCustomizations
+private import LdapInjectionCustomizations::LdapInjection
 
 /**
  * A taint-tracking configuration for detecting LDAP Injections vulnerabilities.

ruby/ql/src/experimental/ldap-injection/LdapInjection.qhelp

@@ -0,0 +1,50 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+If an LDAP query or DN is built using string concatenation or string formatting, and the
+components of the concatenation include user input without any proper sanitization, a user
+is likely to be able to run malicious LDAP queries.
+</p>
+</overview>
+
+<recommendation>
+<p>
+If user input must be included in an LDAP query or DN, it should be escaped to
+avoid a malicious user providing special characters that change the meaning
+of the query.
+</p>
+</recommendation>
+
+<example>
+<p>
+In the following Rails example, an <code>ActionController</code> class
+has a <code>ldap_handler</code> method to handle requests.
+</p>
+
+<p>
+The user and dc is specified using a parameter, <code>user_name</code> and <code>dc</code> provided by
+the client which it then uses to build a LDAP query and DN. This value is accessible using the <code>params</code> method.
+</p>
+
+<p>The first example uses the unsanitized user input directly
+in the search filter and DN for the LDAP query.
+A malicious user could provide special characters to change the meaning of these
+components, and search for a completely different set of values.</p>
+
+<sample src="examples/LdapInjectionBad.rb" />
+
+<p>In the second example, the input provided by the user is sanitized before it is included in the search filter or DN.
+This ensures the meaning of the query cannot be changed by a malicious user.</p>
+
+<sample src="examples/LdapInjectionBad.rb" />
+</example>
+
+<references>
+<li>OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/LDAP_Injection_Prevention_Cheat_Sheet.html">LDAP Injection Prevention Cheat Sheet</a>.</li>
+<li>OWASP: <a href="https://owasp.org/www-community/attacks/LDAP_Injection">LDAP Injection</a>.</li>
+</references>
+</qhelp>

ruby/ql/src/experimental/ldap-injection/examples/LdapInjectionBad.rb

@@ -0,0 +1,21 @@
+require 'net/ldap'
+
+class BadLdapController < ActionController::Base
+  def ldap_handler
+    name = params[:user_name]
+    dc = params[:dc]
+    ldap = Net::LDAP.new(
+        host: 'ldap.example.com',
+        port: 636,
+        encryption: :simple_tls,
+        auth: {
+            method: :simple,
+            username: 'uid=admin,dc=example,dc=com',
+            password: 'adminpassword'
+        }
+    )
+    filter = Net::LDAP::Filter.eq('foo', name)
+    attrs = [name]
+    result = ldap.search(base: "ou=people,dc=#{dc},dc=com", filter: filter, attributes: attrs)
+  end
+end

ruby/ql/src/experimental/ldap-injection/examples/LdapInjectionGood.rb

@@ -0,0 +1,27 @@
+require 'net/ldap'
+
+class GoodLdapController < ActionController::Base
+  def ldap_handler
+    name = params[:user_name]
+    ldap = Net::LDAP.new(
+        host: 'ldap.example.com',
+        port: 636,
+        encryption: :simple_tls,
+        auth: {
+            method: :simple,
+            username: 'uid=admin,dc=example,dc=com',
+            password: 'adminpassword'
+        }
+    )
+    
+    name = if ["admin", "guest"].include? name
+      name
+    else 
+      name = "none"
+    end
+
+    filter = Net::LDAP::Filter.eq('foo', name)
+    attrs = ['dn']
+    result = ldap.search(base: 'ou=people,dc=example,dc=com', filter: filter, attributes: attrs)
+  end
+end