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 all 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 ImproperLdapAuthCustomizations
import ImproperLdapAuth::Flow::PathGraph

from ImproperLdapAuth::Flow::PathNode source, ImproperLdapAuth::Flow::PathNode sink
where ImproperLdapAuth::Flow::flowPath(source, sink)
select sink.getNode(), source, sink, "LDAP binding password depends on a $@.", source.getNode(),
"user-provided value"
@@ -0,0 +1,84 @@
import go
import semmle.go.dataflow.barrierguardutil.RegexpCheck

module ImproperLdapAuth {
/**
* 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 { }

/**
* An empty string.
*/
class EmptyString extends DataFlow::Node {
EmptyString() { this.asExpr().getStringValue() = "" }
}

private predicate equalityAsSanitizerGuard(DataFlow::Node g, Expr e, boolean outcome) {
exists(DataFlow::Node nonConstNode, DataFlow::Node constNode, DataFlow::EqualityTestNode eq |
g = eq and
nonConstNode = eq.getAnOperand() and
not nonConstNode.isConst() and
constNode = eq.getAnOperand() and
constNode.isConst() and
e = nonConstNode.asExpr() and
(
// If `constNode` is not an empty string a comparison is considered a sanitizer
not constNode instanceof EmptyString and outcome = eq.getPolarity()
or
// If `constNode` is an empty string a not comparison is considered a sanitizer
constNode instanceof EmptyString and outcome = eq.getPolarity().booleanNot()
)
)
}

/**
* 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()
}
}

private module Config implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
source instanceof UntrustedFlowSource or source instanceof EmptyString
}

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

predicate isBarrier(DataFlow::Node node) { node instanceof LdapSanitizer }
}

/**
* Tracks taint flow for reasoning about improper ldap auth vulnerabilities
* with sinks which are not sanitized by string comparisons.
*/
module Flow = TaintTracking::Global<Config>;
}
22 changes: 22 additions & 0 deletions go/ql/src/experimental/CWE-287/examples/LdapAuthBad.go
@@ -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