Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Go: Add Improper LDAP Authentication query (CWE-287) #13366

Merged
merged 18 commits into from Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 31 additions & 0 deletions go/ql/src/experimental/CWE-287/ImproperLdapAuth.qhelp
@@ -0,0 +1,31 @@
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
<qhelp>
<overview>
<p>
If an LDAP connection uses user-supplied data as password, anonymous bind could be caused using an empty password
to result in a successful authentication.
</p>
</overview>

<recommendation>
<p>Don't use user-supplied data as password while establishing an LDAP connection.
</p>
</recommendation>

<example>
<p>In the following examples, the code accepts a bind password via a HTTP request in variable <code>
bindPassword</code>. The code builds a LDAP query whose authentication depends on user supplied data.</p>


<sample src="examples/LdapAuthBad.go" />

<p>In the following examples, the code accepts a bind password via a HTTP request in variable <code>
bindPassword</code>. The function ensures that the password provided is not empty before binding. </p>

<sample src="examples/LdapAuthGood.go" />
</example>

<references>
<li>MITRE: <a href="https://cwe.mitre.org/data/definitions/287.html">CWE-287: Improper Authentication</a>.</li>
</references>
</qhelp>
19 changes: 19 additions & 0 deletions go/ql/src/experimental/CWE-287/ImproperLdapAuth.ql
@@ -0,0 +1,19 @@
/**
* @name Improper LDAP Authentication
* @description A user-controlled query carries no authentication
* @kind path-problem
* @problem.severity warning
* @id go/improper-ldap-auth
* @tags security
* experimental
* external/cwe/cwe-287
*/

import go
import ImproperLdapAuth
import DataFlow::PathGraph

from ImproperLdapAuthConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink
where config.hasFlowPath(source, sink)
select sink.getNode(), source, sink, "LDAP binding password depends on a $@.", source.getNode(),
"user-provided value"
83 changes: 83 additions & 0 deletions go/ql/src/experimental/CWE-287/ImproperLdapAuth.qll
@@ -0,0 +1,83 @@
import go
import DataFlow::PathGraph
import semmle.go.dataflow.barrierguardutil.RegexpCheck

/**
* A LDAP connection node.
*/
abstract class LdapConn extends DataFlow::CallNode { }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is ever used. If that is the case then please delete it.


/**
* A sink that is vulnerable to improper LDAP Authentication vulnerabilities.
*/
abstract class LdapAuthSink extends DataFlow::Node { }

/**
* A sanitizer function that prevents improper LDAP Authentication attacks.
*/
abstract class LdapSanitizer extends DataFlow::Node { }

/**
* A vulnerable argument to `go-ldap` or `ldap`'s `bind` function (Only v2).
*/
private class GoLdapBindSink extends LdapAuthSink {
GoLdapBindSink() {
exists(Method meth |
meth.hasQualifiedName("gopkg.in/ldap.v2", "Conn", "Bind") and
this = meth.getACall().getArgument(1)
)
}
}

/**
* A call to a regexp match function, considered as a barrier guard for sanitizing untrusted URLs.
*
* This is overapproximate: we do not attempt to reason about the correctness of the regexp.
*/
class RegexpCheckAsBarrierGuard extends RegexpCheckBarrier, LdapSanitizer { }

private predicate equalityAsSanitizerGuard(DataFlow::Node g, Expr e, boolean outcome) {
exists(DataFlow::Node passwd, DataFlow::EqualityTestNode eq |
g = eq and
passwd = eq.getAnOperand() and
e = passwd.asExpr() and
(
eq.getAnOperand().getStringValue().length() > 0 and outcome = eq.getPolarity()
or
eq.getAnOperand().getStringValue().length() = 0 and
outcome = eq.getPolarity().booleanNot()
)
)
}
maikypedia marked this conversation as resolved.
Show resolved Hide resolved

/**
* An equality check comparing a data-flow node against a constant string, considered as
* a barrier guard for sanitizing untrusted user input.
*/
class EqualityAsSanitizerGuard extends LdapSanitizer {
EqualityAsSanitizerGuard() {
this = DataFlow::BarrierGuard<equalityAsSanitizerGuard/3>::getABarrierNode()
}
}

/**
*/
Fixed Show fixed Hide fixed
maikypedia marked this conversation as resolved.
Show resolved Hide resolved
class EmptyString extends DataFlow::Node {
EmptyString() { this.asExpr().getStringValue() = "" }
}

/**
* A taint-tracking configuration for reasoning about when an `UntrustedFlowSource`
* flows into an argument or field that is vulnerable to Improper LDAP Authentication.
*/
class ImproperLdapAuthConfiguration extends TaintTracking::Configuration {
ImproperLdapAuthConfiguration() { this = "Improper LDAP Auth" }

override predicate isSource(DataFlow::Node source) {
source instanceof UntrustedFlowSource or source instanceof EmptyString
}

override predicate isSink(DataFlow::Node sink) { sink instanceof LdapAuthSink }

override predicate isSanitizer(DataFlow::Node node) { node instanceof LdapSanitizer }
}
maikypedia marked this conversation as resolved.
Show resolved Hide resolved
22 changes: 22 additions & 0 deletions go/ql/src/experimental/CWE-287/examples/LdapAuthBad.go
maikypedia marked this conversation as resolved.
Show resolved Hide resolved
@@ -0,0 +1,22 @@
package main

import (
"fmt"
"log"
)

func bad() interface{} {
bindPassword := req.URL.Query()["password"][0]

// Connect to the LDAP server
l, err := ldap.Dial("tcp", fmt.Sprintf("%s:%d", "ldap.example.com", 389))
if err != nil {
log.Fatalf("Failed to connect to LDAP server: %v", err)
}
defer l.Close()

err = l.Bind("cn=admin,dc=example,dc=com", bindPassword)
if err != nil {
log.Fatalf("LDAP bind failed: %v", err)
}
}
24 changes: 24 additions & 0 deletions go/ql/src/experimental/CWE-287/examples/LdapAuthGood.go
maikypedia marked this conversation as resolved.
Show resolved Hide resolved
@@ -0,0 +1,24 @@
package main

import (
"fmt"
"log"
)

func good() interface{} {
bindPassword := req.URL.Query()["password"][0]

// Connect to the LDAP server
l, err := ldap.Dial("tcp", fmt.Sprintf("%s:%d", "ldap.example.com", 389))
if err != nil {
log.Fatalf("Failed to connect to LDAP server: %v", err)
}
defer l.Close()

if bindPassword != "" {
err = l.Bind("cn=admin,dc=example,dc=com", bindPassword)
if err != nil {
log.Fatalf("LDAP bind failed: %v", err)
}
}
}
14 changes: 14 additions & 0 deletions go/ql/test/experimental/CWE-287/ImproperLdapAuth.expected
@@ -0,0 +1,14 @@
edges
| ImproperLdapAuth.go:18:18:18:24 | selection of URL | ImproperLdapAuth.go:18:18:18:32 | call to Query |
| ImproperLdapAuth.go:18:18:18:32 | call to Query | ImproperLdapAuth.go:28:23:28:34 | bindPassword |
| ImproperLdapAuth.go:87:18:87:19 | "" | ImproperLdapAuth.go:97:23:97:34 | bindPassword |
nodes
| ImproperLdapAuth.go:18:18:18:24 | selection of URL | semmle.label | selection of URL |
| ImproperLdapAuth.go:18:18:18:32 | call to Query | semmle.label | call to Query |
| ImproperLdapAuth.go:28:23:28:34 | bindPassword | semmle.label | bindPassword |
| ImproperLdapAuth.go:87:18:87:19 | "" | semmle.label | "" |
| ImproperLdapAuth.go:97:23:97:34 | bindPassword | semmle.label | bindPassword |
subpaths
#select
| ImproperLdapAuth.go:28:23:28:34 | bindPassword | ImproperLdapAuth.go:18:18:18:24 | selection of URL | ImproperLdapAuth.go:28:23:28:34 | bindPassword | LDAP binding password depends on a $@. | ImproperLdapAuth.go:18:18:18:24 | selection of URL | user-provided value |
| ImproperLdapAuth.go:97:23:97:34 | bindPassword | ImproperLdapAuth.go:87:18:87:19 | "" | ImproperLdapAuth.go:97:23:97:34 | bindPassword | LDAP binding password depends on a $@. | ImproperLdapAuth.go:87:18:87:19 | "" | user-provided value |
108 changes: 108 additions & 0 deletions go/ql/test/experimental/CWE-287/ImproperLdapAuth.go
@@ -0,0 +1,108 @@
package main

//go:generate depstubber -vendor gopkg.in/ldap.v2 Conn Dial

import (
"fmt"
"log"
"net/http"
"regexp"

ldap "gopkg.in/ldap.v2"
)

func bad(w http.ResponseWriter, req *http.Request) (interface{}, error) {
owen-mc marked this conversation as resolved.
Show resolved Hide resolved
ldapServer := "ldap.example.com"
ldapPort := 389
bindDN := "cn=admin,dc=example,dc=com"
bindPassword := req.URL.Query()["password"][0]

// Connect to the LDAP server
l, err := ldap.Dial("tcp", fmt.Sprintf("%s:%d", ldapServer, ldapPort))
if err != nil {
return fmt.Errorf("Failed to connect to LDAP server: %v", err), err
}
defer l.Close()

// BAD: user input is not sanetized
err = l.Bind(bindDN, bindPassword)
if err != nil {
return fmt.Errorf("LDAP bind failed: %v", err), err
}
return nil, nil
}

func good1(w http.ResponseWriter, req *http.Request) (interface{}, error) {
ldapServer := "ldap.example.com"
ldapPort := 389
bindDN := "cn=admin,dc=example,dc=com"
bindPassword := req.URL.Query()["password"][0]

// Connect to the LDAP server
l, err := ldap.Dial("tcp", fmt.Sprintf("%s:%d", ldapServer, ldapPort))
if err != nil {
return fmt.Errorf("Failed to connect to LDAP server: %v", err), err
}
defer l.Close()

hasEmptyInput, _ := regexp.MatchString("^\\s*$", bindPassword)

// GOOD : bindPassword is not empty
if !hasEmptyInput {
l.Bind(bindDN, bindPassword)
}
if err != nil {
return fmt.Errorf("LDAP bind failed: %v", err), err
}
return nil, nil
}

func good2(w http.ResponseWriter, req *http.Request) (interface{}, error) {
ldapServer := "ldap.example.com"
ldapPort := 389
bindDN := "cn=admin,dc=example,dc=com"
bindPassword := req.URL.Query()["password"][0]

// Connect to the LDAP server
l, err := ldap.Dial("tcp", fmt.Sprintf("%s:%d", ldapServer, ldapPort))
if err != nil {
return fmt.Errorf("Failed to connect to LDAP server: %v", err), err
}
defer l.Close()

// GOOD : bindPassword is not empty
if bindPassword != "" {
l.Bind(bindDN, bindPassword)
return nil, err
}
return nil, nil
}

func bad2(req *http.Request) {
// LDAP server details
ldapServer := "ldap.example.com"
ldapPort := 389
bindDN := "cn=admin,dc=example,dc=com"
// BAD : empty password
bindPassword := ""

// Connect to the LDAP server
l, err := ldap.Dial("tcp", fmt.Sprintf("%s:%d", ldapServer, ldapPort))
if err != nil {
log.Fatalf("Failed to connect to LDAP server: %v", err)
}
defer l.Close()

// BAD : bindPassword is empty
err = l.Bind(bindDN, bindPassword)
if err != nil {
log.Fatalf("LDAP bind failed: %v", err)
}
}

func main() {
bad(nil, nil)
good1(nil, nil)
good2(nil, nil)
bad2(nil)
}
1 change: 1 addition & 0 deletions go/ql/test/experimental/CWE-287/ImproperLdapAuth.qlref
@@ -0,0 +1 @@
experimental/CWE-287/ImproperLdapAuth.ql
7 changes: 7 additions & 0 deletions go/ql/test/experimental/CWE-287/go.mod
@@ -0,0 +1,7 @@
module main

go 1.19

require gopkg.in/ldap.v2 v2.5.1

require gopkg.in/asn1-ber.v1 v1.0.0-20181015200546-f715ec2f112d // indirect