Skip to content

Commit d931ade

Browse files
owen-mcowen-mc
authored
Merge pull request github#13366 from maikypedia/maikypedia/go-ldap-improper-auth
Go: Add Improper LDAP Authentication query (CWE-287)
2 parents dc4dda1 + 6e533c6 commit d931ade

File tree

11 files changed

+501
-0
lines changed

11 files changed

+501
-0
lines changed

‎go/ql/src/experimental/CWE-287/ImproperLdapAuth.qhelp

Lines changed: 31 additions & 0 deletions
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>

‎go/ql/src/experimental/CWE-287/ImproperLdapAuth.ql

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/**
2+
* @name Improper LDAP Authentication
3+
* @description A user-controlled query carries no authentication
4+
* @kind path-problem
5+
* @problem.severity warning
6+
* @id go/improper-ldap-auth
7+
* @tags security
8+
* experimental
9+
* external/cwe/cwe-287
10+
*/
11+
12+
import go
13+
import ImproperLdapAuthCustomizations
14+
import ImproperLdapAuth::Flow::PathGraph
15+
16+
from ImproperLdapAuth::Flow::PathNode source, ImproperLdapAuth::Flow::PathNode sink
17+
where ImproperLdapAuth::Flow::flowPath(source, sink)
18+
select sink.getNode(), source, sink, "LDAP binding password depends on a $@.", source.getNode(),
19+
"user-provided value"

‎go/ql/src/experimental/CWE-287/ImproperLdapAuthCustomizations.qll

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
import go
2+
import semmle.go.dataflow.barrierguardutil.RegexpCheck
3+
4+
module ImproperLdapAuth {
5+
/**
6+
* A sink that is vulnerable to improper LDAP Authentication vulnerabilities.
7+
*/
8+
abstract class LdapAuthSink extends DataFlow::Node { }
9+
10+
/**
11+
* A sanitizer function that prevents improper LDAP Authentication attacks.
12+
*/
13+
abstract class LdapSanitizer extends DataFlow::Node { }
14+
15+
/**
16+
* A vulnerable argument to `go-ldap` or `ldap`'s `bind` function (Only v2).
17+
*/
18+
private class GoLdapBindSink extends LdapAuthSink {
19+
GoLdapBindSink() {
20+
exists(Method meth |
21+
meth.hasQualifiedName("gopkg.in/ldap.v2", "Conn", "Bind") and
22+
this = meth.getACall().getArgument(1)
23+
)
24+
}
25+
}
26+
27+
/**
28+
* A call to a regexp match function, considered as a barrier guard for sanitizing untrusted URLs.
29+
*
30+
* This is overapproximate: we do not attempt to reason about the correctness of the regexp.
31+
*/
32+
class RegexpCheckAsBarrierGuard extends RegexpCheckBarrier, LdapSanitizer { }
33+
34+
/**
35+
* An empty string.
36+
*/
37+
class EmptyString extends DataFlow::Node {
38+
EmptyString() { this.asExpr().getStringValue() = "" }
39+
}
40+
41+
private predicate equalityAsSanitizerGuard(DataFlow::Node g, Expr e, boolean outcome) {
42+
exists(DataFlow::Node nonConstNode, DataFlow::Node constNode, DataFlow::EqualityTestNode eq |
43+
g = eq and
44+
nonConstNode = eq.getAnOperand() and
45+
not nonConstNode.isConst() and
46+
constNode = eq.getAnOperand() and
47+
constNode.isConst() and
48+
e = nonConstNode.asExpr() and
49+
(
50+
// If `constNode` is not an empty string a comparison is considered a sanitizer
51+
not constNode instanceof EmptyString and outcome = eq.getPolarity()
52+
or
53+
// If `constNode` is an empty string a not comparison is considered a sanitizer
54+
constNode instanceof EmptyString and outcome = eq.getPolarity().booleanNot()
55+
)
56+
)
57+
}
58+
59+
/**
60+
* An equality check comparing a data-flow node against a constant string, considered as
61+
* a barrier guard for sanitizing untrusted user input.
62+
*/
63+
class EqualityAsSanitizerGuard extends LdapSanitizer {
64+
EqualityAsSanitizerGuard() {
65+
this = DataFlow::BarrierGuard<equalityAsSanitizerGuard/3>::getABarrierNode()
66+
}
67+
}
68+
69+
private module Config implements DataFlow::ConfigSig {
70+
predicate isSource(DataFlow::Node source) {
71+
source instanceof UntrustedFlowSource or source instanceof EmptyString
72+
}
73+
74+
predicate isSink(DataFlow::Node sink) { sink instanceof LdapAuthSink }
75+
76+
predicate isBarrier(DataFlow::Node node) { node instanceof LdapSanitizer }
77+
}
78+
79+
/**
80+
* Tracks taint flow for reasoning about improper ldap auth vulnerabilities
81+
* with sinks which are not sanitized by string comparisons.
82+
*/
83+
module Flow = TaintTracking::Global<Config>;
84+
}

‎go/ql/src/experimental/CWE-287/examples/LdapAuthBad.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package main
2+
3+
import (
4+
"fmt"
5+
"log"
6+
)
7+
8+
func bad() interface{} {
9+
bindPassword := req.URL.Query()["password"][0]
10+
11+
// Connect to the LDAP server
12+
l, err := ldap.Dial("tcp", fmt.Sprintf("%s:%d", "ldap.example.com", 389))
13+
if err != nil {
14+
log.Fatalf("Failed to connect to LDAP server: %v", err)
15+
}
16+
defer l.Close()
17+
18+
err = l.Bind("cn=admin,dc=example,dc=com", bindPassword)
19+
if err != nil {
20+
log.Fatalf("LDAP bind failed: %v", err)
21+
}
22+
}

‎go/ql/src/experimental/CWE-287/examples/LdapAuthGood.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package main
2+
3+
import (
4+
"fmt"
5+
"log"
6+
)
7+
8+
func good() interface{} {
9+
bindPassword := req.URL.Query()["password"][0]
10+
11+
// Connect to the LDAP server
12+
l, err := ldap.Dial("tcp", fmt.Sprintf("%s:%d", "ldap.example.com", 389))
13+
if err != nil {
14+
log.Fatalf("Failed to connect to LDAP server: %v", err)
15+
}
16+
defer l.Close()
17+
18+
if bindPassword != "" {
19+
err = l.Bind("cn=admin,dc=example,dc=com", bindPassword)
20+
if err != nil {
21+
log.Fatalf("LDAP bind failed: %v", err)
22+
}
23+
}
24+
}

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
edges
2+
| ImproperLdapAuth.go:18:18:18:24 | selection of URL | ImproperLdapAuth.go:18:18:18:32 | call to Query |
3+
| ImproperLdapAuth.go:18:18:18:32 | call to Query | ImproperLdapAuth.go:28:23:28:34 | bindPassword |
4+
| ImproperLdapAuth.go:87:18:87:19 | "" | ImproperLdapAuth.go:97:23:97:34 | bindPassword |
5+
nodes
6+
| ImproperLdapAuth.go:18:18:18:24 | selection of URL | semmle.label | selection of URL |
7+
| ImproperLdapAuth.go:18:18:18:32 | call to Query | semmle.label | call to Query |
8+
| ImproperLdapAuth.go:28:23:28:34 | bindPassword | semmle.label | bindPassword |
9+
| ImproperLdapAuth.go:87:18:87:19 | "" | semmle.label | "" |
10+
| ImproperLdapAuth.go:97:23:97:34 | bindPassword | semmle.label | bindPassword |
11+
subpaths
12+
#select
13+
| 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 |
14+
| 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 |

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

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
package main
2+
3+
//go:generate depstubber -vendor gopkg.in/ldap.v2 Conn Dial
4+
5+
import (
6+
"fmt"
7+
"log"
8+
"net/http"
9+
"regexp"
10+
11+
ldap "gopkg.in/ldap.v2"
12+
)
13+
14+
func bad(w http.ResponseWriter, req *http.Request) (interface{}, error) {
15+
ldapServer := "ldap.example.com"
16+
ldapPort := 389
17+
bindDN := "cn=admin,dc=example,dc=com"
18+
bindPassword := req.URL.Query()["password"][0]
19+
20+
// Connect to the LDAP server
21+
l, err := ldap.Dial("tcp", fmt.Sprintf("%s:%d", ldapServer, ldapPort))
22+
if err != nil {
23+
return fmt.Errorf("Failed to connect to LDAP server: %v", err), err
24+
}
25+
defer l.Close()
26+
27+
// BAD: user input is not sanetized
28+
err = l.Bind(bindDN, bindPassword)
29+
if err != nil {
30+
return fmt.Errorf("LDAP bind failed: %v", err), err
31+
}
32+
return nil, nil
33+
}
34+
35+
func good1(w http.ResponseWriter, req *http.Request) (interface{}, error) {
36+
ldapServer := "ldap.example.com"
37+
ldapPort := 389
38+
bindDN := "cn=admin,dc=example,dc=com"
39+
bindPassword := req.URL.Query()["password"][0]
40+
41+
// Connect to the LDAP server
42+
l, err := ldap.Dial("tcp", fmt.Sprintf("%s:%d", ldapServer, ldapPort))
43+
if err != nil {
44+
return fmt.Errorf("Failed to connect to LDAP server: %v", err), err
45+
}
46+
defer l.Close()
47+
48+
hasEmptyInput, _ := regexp.MatchString("^\\s*$", bindPassword)
49+
50+
// GOOD : bindPassword is not empty
51+
if !hasEmptyInput {
52+
l.Bind(bindDN, bindPassword)
53+
}
54+
if err != nil {
55+
return fmt.Errorf("LDAP bind failed: %v", err), err
56+
}
57+
return nil, nil
58+
}
59+
60+
func good2(w http.ResponseWriter, req *http.Request) (interface{}, error) {
61+
ldapServer := "ldap.example.com"
62+
ldapPort := 389
63+
bindDN := "cn=admin,dc=example,dc=com"
64+
bindPassword := req.URL.Query()["password"][0]
65+
66+
// Connect to the LDAP server
67+
l, err := ldap.Dial("tcp", fmt.Sprintf("%s:%d", ldapServer, ldapPort))
68+
if err != nil {
69+
return fmt.Errorf("Failed to connect to LDAP server: %v", err), err
70+
}
71+
defer l.Close()
72+
73+
// GOOD : bindPassword is not empty
74+
if bindPassword != "" {
75+
l.Bind(bindDN, bindPassword)
76+
return nil, err
77+
}
78+
return nil, nil
79+
}
80+
81+
func bad2(req *http.Request) {
82+
// LDAP server details
83+
ldapServer := "ldap.example.com"
84+
ldapPort := 389
85+
bindDN := "cn=admin,dc=example,dc=com"
86+
// BAD : empty password
87+
bindPassword := ""
88+
89+
// Connect to the LDAP server
90+
l, err := ldap.Dial("tcp", fmt.Sprintf("%s:%d", ldapServer, ldapPort))
91+
if err != nil {
92+
log.Fatalf("Failed to connect to LDAP server: %v", err)
93+
}
94+
defer l.Close()
95+
96+
// BAD : bindPassword is empty
97+
err = l.Bind(bindDN, bindPassword)
98+
if err != nil {
99+
log.Fatalf("LDAP bind failed: %v", err)
100+
}
101+
}
102+
103+
func main() {
104+
bad(nil, nil)
105+
good1(nil, nil)
106+
good2(nil, nil)
107+
bad2(nil)
108+
}

‎go/ql/test/experimental/CWE-287/ImproperLdapAuth.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/CWE-287/ImproperLdapAuth.ql

‎go/ql/test/experimental/CWE-287/go.mod

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
module main
2+
3+
go 1.19
4+
5+
require gopkg.in/ldap.v2 v2.5.1
6+
7+
require gopkg.in/asn1-ber.v1 v1.0.0-20181015200546-f715ec2f112d // indirect

0 commit comments

Comments
 (0)