Skip to content

Commit 5f5e818

Browse files
authored
Merge commit from fork
auth/ldap: Fix GHSA-5835-4gvc-32pc
2 parents b7316d9 + 6a06337 commit 5f5e818

5 files changed

Lines changed: 192 additions & 4 deletions

File tree

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ require (
107107
github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
108108
github.com/hashicorp/go-retryablehttp v0.7.7 // indirect
109109
github.com/hashicorp/hcl v1.0.0 // indirect
110+
github.com/jimlambrt/gldap v0.1.14 // indirect
110111
github.com/jmespath/go-jmespath v0.4.0 // indirect
111112
github.com/josharian/intern v1.0.0 // indirect
112113
github.com/klauspost/compress v1.17.11 // indirect

go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,8 @@ github.com/jcmturner/gokrb5/v8 v8.4.4 h1:x1Sv4HaTpepFkXbt2IkL29DXRf8sOfZXo8eRKh6
488488
github.com/jcmturner/gokrb5/v8 v8.4.4/go.mod h1:1btQEpgT6k+unzCwX1KdWMEwPPkkgBtP+F6aCACiMrs=
489489
github.com/jcmturner/rpc/v2 v2.0.3 h1:7FXXj8Ti1IaVFpSAziCZWNzbNuZmnvw/i6CqLNdWfZY=
490490
github.com/jcmturner/rpc/v2 v2.0.3/go.mod h1:VUJYCIDm3PVOEHw8sgt091/20OJjskO/YJki3ELg/Hc=
491+
github.com/jimlambrt/gldap v0.1.14 h1:InG9kldhIu6OoQK0hvfkW1Lqpc5eLJhxiiDTNmRnrDM=
492+
github.com/jimlambrt/gldap v0.1.14/go.mod h1:yobW9JIAmqe23dVNOaMWewPaff6jGaHgYjspPIIgYmg=
491493
github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k=
492494
github.com/jmespath/go-jmespath v0.4.0 h1:BEgLn5cpjn8UN1mAw4NjwDrS35OdebyEtFe+9YPoQUg=
493495
github.com/jmespath/go-jmespath v0.4.0/go.mod h1:T8mJZnbsbmF+m6zOOFylbeCJqk5+pHWvzYPziyZiYoo=

internal/auth/ldap/ldap.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ func (a *Auth) Lookup(_ context.Context, username string) (string, bool, error)
225225
req := ldap.NewSearchRequest(
226226
a.baseDN, ldap.ScopeWholeSubtree, ldap.NeverDerefAliases,
227227
2, 0, false,
228-
strings.ReplaceAll(a.filterTemplate, "{username}", username),
228+
strings.ReplaceAll(a.filterTemplate, "{username}", ldap.EscapeFilter(username)),
229229
[]string{"dn"}, nil)
230230
res, err := conn.Search(req)
231231
if err != nil {
@@ -252,12 +252,12 @@ func (a *Auth) AuthPlain(username, password string) error {
252252

253253
var userDN string
254254
if a.dnTemplate != "" {
255-
userDN = strings.ReplaceAll(a.dnTemplate, "{username}", username)
255+
userDN = strings.ReplaceAll(a.dnTemplate, "{username}", ldap.EscapeDN(username))
256256
} else {
257257
req := ldap.NewSearchRequest(
258258
a.baseDN, ldap.ScopeWholeSubtree, ldap.NeverDerefAliases,
259259
2, 0, false,
260-
strings.ReplaceAll(a.filterTemplate, "{username}", username),
260+
strings.ReplaceAll(a.filterTemplate, "{username}", ldap.EscapeFilter(username)),
261261
[]string{"dn"}, nil)
262262
res, err := conn.Search(req)
263263
if err != nil {

tests/conn.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ func (c *Conn) SMTPPlainAuth(username, password string, expectOk bool) {
211211
if expectOk {
212212
c.ExpectPattern("235 *")
213213
} else {
214-
c.ExpectPattern("*")
214+
c.ExpectPattern("5*")
215215
}
216216
}
217217

@@ -282,6 +282,13 @@ func (c *Conn) Close() error {
282282
return c.Conn.Close()
283283
}
284284

285+
func (c *Conn) MustClose() {
286+
c.T.Helper()
287+
if err := c.Close(); err != nil {
288+
c.fatal("Close: %v", err)
289+
}
290+
}
291+
285292
func (c *Conn) Rebind(subtest *T) *Conn {
286293
cpy := *c
287294
cpy.T = subtest

tests/ghsa_5835_4gvc_32pc_test.go

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
//go:build integration
2+
3+
package tests_test
4+
5+
import (
6+
"strconv"
7+
"testing"
8+
"time"
9+
10+
"github.com/foxcpp/maddy/tests"
11+
"github.com/jimlambrt/gldap"
12+
"github.com/stretchr/testify/require"
13+
)
14+
15+
type searchEntry struct {
16+
dn string
17+
options []gldap.Option
18+
}
19+
20+
type MockLDAP struct {
21+
T *testing.T
22+
SearchEntries map[string][]searchEntry
23+
AllowedBinds map[string]string
24+
}
25+
26+
func (ml *MockLDAP) HandleBind(w *gldap.ResponseWriter, r *gldap.Request) {
27+
resp := r.NewBindResponse(
28+
gldap.WithResponseCode(gldap.ResultInvalidCredentials),
29+
)
30+
31+
m, err := r.GetSimpleBindMessage()
32+
if err != nil {
33+
require.NoError(ml.T, w.Write(resp))
34+
return
35+
}
36+
37+
pass, ok := ml.AllowedBinds[m.UserName]
38+
if ok && pass == string(m.Password) {
39+
resp.SetResultCode(gldap.ResultSuccess)
40+
require.NoError(ml.T, w.Write(resp))
41+
}
42+
43+
require.NoError(ml.T, w.Write(resp))
44+
}
45+
46+
func (ml *MockLDAP) HandleSearch(w *gldap.ResponseWriter, r *gldap.Request) {
47+
resp := r.NewSearchDoneResponse()
48+
m, err := r.GetSearchMessage()
49+
if err != nil {
50+
ml.T.Logf("not a search message: %s", err)
51+
require.NoError(ml.T, w.Write(resp))
52+
return
53+
}
54+
ml.T.Logf("search base dn: %s", m.BaseDN)
55+
ml.T.Logf("search scope: %d", m.Scope)
56+
ml.T.Logf("search filter: %s", m.Filter)
57+
58+
entries := ml.SearchEntries[m.Filter]
59+
for _, entry := range entries {
60+
ldapEntry := r.NewSearchResponseEntry(entry.dn, entry.options...)
61+
require.NoError(ml.T, w.Write(ldapEntry))
62+
}
63+
64+
resp.SetResultCode(gldap.ResultSuccess)
65+
require.NoError(ml.T, w.Write(resp))
66+
}
67+
68+
func (ml *MockLDAP) Run(address string) {
69+
s, err := gldap.NewServer()
70+
if err != nil {
71+
ml.T.Fatalf("unable to create server: %s", err.Error())
72+
}
73+
74+
// create a router and add a bind handler
75+
r, err := gldap.NewMux()
76+
if err != nil {
77+
ml.T.Fatalf("unable to create router: %s", err.Error())
78+
}
79+
require.NoError(ml.T, r.Bind(ml.HandleBind))
80+
require.NoError(ml.T, r.Search(ml.HandleSearch))
81+
require.NoError(ml.T, s.Router(r))
82+
go func() {
83+
require.NoError(ml.T, s.Run(address))
84+
}()
85+
ml.T.Cleanup(func() {
86+
require.NoError(ml.T, s.Stop())
87+
})
88+
89+
for !s.Ready() {
90+
ml.T.Log("Waiting for server to start")
91+
time.Sleep(100 * time.Millisecond)
92+
}
93+
}
94+
95+
func TestLDAPInjectionFilter(tt *testing.T) {
96+
tt.Parallel()
97+
t := tests.NewT(tt)
98+
99+
ldapPort := t.Port("ldap")
100+
101+
ldapSrv := &MockLDAP{
102+
T: tt,
103+
AllowedBinds: map[string]string{
104+
"DC=com,CN=bob": "bob_pass",
105+
"DC=com,CN=alice": "alice_pass",
106+
},
107+
SearchEntries: map[string][]searchEntry{
108+
"(&(objectClass=inetOrgPerson)(uid=alice))": {
109+
{
110+
dn: "DC=com,CN=alice",
111+
options: []gldap.Option{
112+
gldap.WithAttributes(map[string][]string{
113+
"objectClass": {"inetOrgPerson"},
114+
"uid": {"alice"},
115+
"description": {"prefix_test"},
116+
}),
117+
},
118+
},
119+
},
120+
"(&(objectClass=inetOrgPerson)(uid=bob))": {
121+
{
122+
dn: "DC=com,CN=bob",
123+
options: []gldap.Option{
124+
gldap.WithAttributes(map[string][]string{
125+
"objectClass": {"inetOrgPerson"},
126+
"uid": {"bob"},
127+
"description": {"prefix_test"},
128+
}),
129+
},
130+
},
131+
},
132+
"(&(objectClass=inetOrgPerson)(uid=bob)(description=prefix*))": {
133+
{
134+
dn: "DC=com,CN=bob",
135+
options: []gldap.Option{
136+
gldap.WithAttributes(map[string][]string{
137+
"objectClass": {"inetOrgPerson"},
138+
"uid": {"bob"},
139+
"description": {"prefix_test"},
140+
}),
141+
},
142+
},
143+
},
144+
},
145+
}
146+
ldapSrv.Run(":" + strconv.Itoa(int(ldapPort)))
147+
148+
t.Port("smtp")
149+
t.DNS(nil)
150+
t.Config(`
151+
hostname mx.maddy.test
152+
tls off
153+
154+
auth.ldap ldap_auth {
155+
urls ldap://127.0.0.1:{env:TEST_PORT_ldap}
156+
bind plain "DC=com,CN=bob" "bob_pass"
157+
base_dn "DC=com"
158+
filter "(&(objectClass=inetOrgPerson)(uid={username}))"
159+
}
160+
161+
submission tcp://0.0.0.0:{env:TEST_PORT_smtp} {
162+
auth &ldap_auth
163+
deliver_to dummy
164+
}
165+
`)
166+
t.Run(1)
167+
defer t.Close()
168+
169+
smtpConn := t.Conn("smtp")
170+
defer smtpConn.MustClose()
171+
smtpConn.SMTPNegotation("clieht.maddy.test", nil, nil)
172+
smtpConn.SMTPPlainAuth("alice", "alice_pass", true)
173+
174+
smtpConn2 := t.Conn("smtp")
175+
defer smtpConn2.MustClose()
176+
smtpConn2.SMTPNegotation("clieht.maddy.test", nil, nil)
177+
smtpConn2.SMTPPlainAuth("bob)(description=prefix*", "bob_pass", false)
178+
}

0 commit comments

Comments
 (0)