Skip to content

Commit bf9d0b9

Browse files
committed
Add Improper LDAP Auth Query (CWE-287)
1 parent 16bc584 commit bf9d0b9

File tree

9 files changed

+240
-0
lines changed

9 files changed

+240
-0
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
<overview>
4+
<p>
5+
If an LDAP connection uses user-supplied data as password, anonymous bind could be caused using an empty password
6+
to result in a successful authentication.
7+
</p>
8+
</overview>
9+
10+
<recommendation>
11+
<p>Don't use user-supplied data as password while establishing an LDAP connection.
12+
</p>
13+
</recommendation>
14+
15+
<example>
16+
<p>In the following examples, the code accepts a bind password via a HTTP request in variable <code>
17+
bindPassword</code>. The code builds a LDAP query whose authentication depends on user supplied data.</p>
18+
19+
20+
<sample src="examples/LdapAuthBad.go" />
21+
22+
<p>In the following examples, the code accepts a bind password via a HTTP request in variable <code>
23+
bindPassword</code>. The function ensures that the password provided is not empty before binding. </p>
24+
25+
<sample src="examples/LdapAuthGood.go" />
26+
</example>
27+
28+
<references>
29+
<li>MITRE: <a href="https://cwe.mitre.org/data/definitions/287.html">CWE-287: Improper Authentication</a>.</li>
30+
</references>
31+
</qhelp>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/**
2+
* @name LDAP query built from user-controlled sources
3+
* @description Building an LDAP query from user-controlled sources is vulnerable to insertion of
4+
* malicious LDAP code by the user.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @id go/ldap-injection
8+
* @tags security
9+
* experimental
10+
* external/cwe/cwe-90
11+
*/
12+
13+
import go
14+
import ImproperLdapAuth
15+
import DataFlow::PathGraph
16+
17+
from ImproperLdapAuthConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink
18+
where config.hasFlowPath(source, sink)
19+
select sink.getNode(), source, sink, "LDAP binding password depends on a $@.", source.getNode(),
20+
"user-provided value"
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
import go
2+
import DataFlow::PathGraph
3+
import semmle.go.dataflow.barrierguardutil.RegexpCheck
4+
5+
/**
6+
* A LDAP connection node.
7+
*/
8+
abstract class LdapConn extends DataFlow::CallNode { }
9+
10+
/**
11+
* A sink that is vulnerable to improper LDAP Authentication vulnerabilities.
12+
*/
13+
abstract class LdapAuthSink extends DataFlow::Node { }
14+
15+
/**
16+
* A sanitizer function that prevents improper LDAP Authentication attacks.
17+
*/
18+
abstract class LdapSanitizer extends DataFlow::Node { }
19+
20+
/**
21+
* A vulnerable argument to `go-ldap` or `ldap`'s `NewSearchRequest` function.
22+
*/
23+
private class GoLdapBindSink extends LdapAuthSink {
24+
GoLdapBindSink() {
25+
exists(Method meth, string base, string t, string m |
26+
t = ["Conn"] and
27+
meth.hasQualifiedName([
28+
"github.com/go-ldap/ldap", "github.com/go-ldap/ldap/v3", "gopkg.in/ldap.v2",
29+
"gopkg.in/ldap.v3"
30+
], t, m) and
31+
this = meth.getACall().getArgument(1)
32+
|
33+
base = ["Bind"] and m = base
34+
)
35+
}
36+
}
37+
38+
/**
39+
* A call to a regexp match function, considered as a barrier guard for sanitizing untrusted URLs.
40+
*
41+
* This is overapproximate: we do not attempt to reason about the correctness of the regexp.
42+
*/
43+
class RegexpCheckAsBarrierGuard extends RegexpCheckBarrier, LdapSanitizer { }
44+
45+
private predicate equalityAsSanitizerGuard(DataFlow::Node g, Expr e, boolean outcome) {
46+
exists(DataFlow::Node passwd, DataFlow::EqualityTestNode eq |
47+
g = eq and
48+
exists(eq.getAnOperand().getStringValue()) and
49+
passwd = eq.getAnOperand() and
50+
e = passwd.asExpr() and
51+
outcome = true
52+
)
53+
}
54+
55+
/**
56+
* An equality check comparing a data-flow node against a constant string, considered as
57+
* a barrier guard for sanitizing untrusted user input.
58+
*/
59+
class EqualityAsSanitizerGuard extends LdapSanitizer {
60+
EqualityAsSanitizerGuard() {
61+
this = DataFlow::BarrierGuard<equalityAsSanitizerGuard/3>::getABarrierNode()
62+
}
63+
}
64+
65+
/**
66+
* A taint-tracking configuration for reasoning about when an `UntrustedFlowSource`
67+
* flows into an argument or field that is vulnerable to LDAP injection.
68+
*/
69+
class ImproperLdapAuthConfiguration extends TaintTracking::Configuration {
70+
ImproperLdapAuthConfiguration() { this = "Improper LDAP Auth" }
71+
72+
override predicate isSource(DataFlow::Node source) { source instanceof UntrustedFlowSource }
73+
74+
override predicate isSink(DataFlow::Node sink) { sink instanceof LdapAuthSink }
75+
76+
override predicate isSanitizer(DataFlow::Node node) { node instanceof LdapSanitizer }
77+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
func bad() interface{} {
2+
bindPassword := req.URL.Query()["password"][0]
3+
4+
// Connect to the LDAP server
5+
l, err := ldap.Dial("tcp", fmt.Sprintf("%s:%d", "ldap.example.com", 389))
6+
if err != nil {
7+
log.Fatalf("Failed to connect to LDAP server: %v", err)
8+
}
9+
defer l.Close()
10+
11+
err = l.Bind("cn=admin,dc=example,dc=com", bindPassword)
12+
if err != nil {
13+
log.Fatalf("LDAP bind failed: %v", err)
14+
}
15+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
func good() interface{} {
2+
bindPassword := req.URL.Query()["password"][0]
3+
4+
// Connect to the LDAP server
5+
l, err := ldap.Dial("tcp", fmt.Sprintf("%s:%d", "ldap.example.com", 389))
6+
if err != nil {
7+
log.Fatalf("Failed to connect to LDAP server: %v", err)
8+
}
9+
defer l.Close()
10+
11+
if bindPassword != "" {
12+
l.Bind(bindDN, bindPassword)
13+
if err != nil {
14+
log.Fatalf("LDAP bind failed: %v", err)
15+
}
16+
}
17+
}

go/ql/test/experimental/CWE-287/ImproperLdapAuth.expected

Whitespace-only changes.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
package main
2+
3+
import (
4+
"fmt"
5+
"log"
6+
"net/http"
7+
"regexp"
8+
9+
ldap "github.com/go-ldap/ldap/v3"
10+
)
11+
12+
func bad(w http.ResponseWriter, req *http.Request) (interface{}, error) {
13+
ldapServer := "ldap.example.com"
14+
ldapPort := 389
15+
bindDN := "cn=admin,dc=example,dc=com"
16+
bindPassword := req.URL.Query()["password"][0]
17+
18+
// Connect to the LDAP server
19+
l, err := ldap.Dial("tcp", fmt.Sprintf("%s:%d", ldapServer, ldapPort))
20+
if err != nil {
21+
log.Fatalf("Failed to connect to LDAP server: %v", err)
22+
}
23+
defer l.Close()
24+
25+
// BAD: user input is not sanetized
26+
err = l.Bind(bindDN, bindPassword)
27+
if err != nil {
28+
log.Fatalf("LDAP bind failed: %v", err)
29+
}
30+
}
31+
32+
func good1(w http.ResponseWriter, req *http.Request) (interface{}, error) {
33+
ldapServer := "ldap.example.com"
34+
ldapPort := 389
35+
bindDN := "cn=admin,dc=example,dc=com"
36+
bindPassword := req.URL.Query()["password"][0]
37+
38+
// Connect to the LDAP server
39+
l, err := ldap.Dial("tcp", fmt.Sprintf("%s:%d", ldapServer, ldapPort))
40+
if err != nil {
41+
log.Fatalf("Failed to connect to LDAP server: %v", err)
42+
}
43+
defer l.Close()
44+
45+
hasEmptyInput, _ := regexp.MatchString("^\\s*$", bindPassword)
46+
47+
// GOOD : bindPassword is not empty
48+
if !hasEmptyInput {
49+
l.Bind(bindDN, bindPassword)
50+
}
51+
}
52+
53+
func good2(w http.ResponseWriter, req *http.Request) (interface{}, error) {
54+
ldapServer := "ldap.example.com"
55+
ldapPort := 389
56+
bindDN := "cn=admin,dc=example,dc=com"
57+
bindPassword := req.URL.Query()["password"][0]
58+
59+
// Connect to the LDAP server
60+
l, err := ldap.Dial("tcp", fmt.Sprintf("%s:%d", ldapServer, ldapPort))
61+
if err != nil {
62+
log.Fatalf("Failed to connect to LDAP server: %v", err)
63+
}
64+
defer l.Close()
65+
66+
// GOOD : bindPassword is not empty
67+
if bindPassword != "" {
68+
l.Bind(bindDN, bindPassword)
69+
}
70+
}
71+
72+
func main() {
73+
bad(nil, nil)
74+
good1(nil, nil)
75+
good2(nil, nil)
76+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/CWE-287/ImproperLdapAuth.ql
+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
module github.com/go-ldap/ldap/v3
2+
3+
go 1.19

0 commit comments

Comments
 (0)