Skip to content

Commit 3bbbfb1

Browse files
TomSellersTomSellers
authored
Address panic in GetLDAPError, add fuzzer (#582)
* Address panic in GetLDAPError, add fuzzer * adjust test logic * remove unused struct field * update make fuzz * Add coverage for nil matchDN in packet passed to GetLDAPError
1 parent 539d8f5 commit 3bbbfb1

4 files changed

Lines changed: 147 additions & 64 deletions

File tree

‎Makefile‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
default: fmt vet lint build test
44

5-
CONTAINER_CMD := $(shell command -v docker 2>/dev/null || shell command -v podman 2>/dev/null)
5+
CONTAINER_CMD := $(shell (command -v docker 2>/dev/null || command -v podman 2>/dev/null))
66
ifeq ($(CONTAINER_CMD),)
77
$(error Neither podman nor docker found in PATH)
88
endif
@@ -53,9 +53,7 @@ test:
5353
go test -v -cover -race -count=1 .
5454

5555
fuzz:
56-
go test -fuzz=FuzzParseDN -fuzztime=600s .
57-
go test -fuzz=FuzzDecodeEscapedSymbols -fuzztime=600s .
58-
go test -fuzz=FuzzEscapeDN -fuzztime=600s .
56+
(cd v3 && go test -fuzz=FuzzGetLDAPError -fuzztime=600s .)
5957

6058
# Capture output and force failure when there is non-empty output
6159
fmt:

‎v3/error.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,13 +210,20 @@ func GetLDAPError(packet *ber.Packet) error {
210210
}
211211
if response.ClassType == ber.ClassApplication && response.TagType == ber.TypeConstructed && len(response.Children) >= 3 {
212212
if ber.Type(response.Children[0].Tag) == ber.Type(ber.TagInteger) || ber.Type(response.Children[0].Tag) == ber.Type(ber.TagEnumerated) {
213+
if response.Children[0].Value == nil {
214+
return &Error{ResultCode: ErrorNetwork, Err: fmt.Errorf("Invalid result code in packet"), Packet: packet}
215+
}
216+
213217
resultCode := uint16(response.Children[0].Value.(int64))
214218
if resultCode == 0 { // No error
215219
return nil
216220
}
217221

218222
if ber.Type(response.Children[1].Tag) == ber.Type(ber.TagOctetString) &&
219223
ber.Type(response.Children[2].Tag) == ber.Type(ber.TagOctetString) {
224+
if response.Children[1].Value == nil {
225+
return &Error{ResultCode: ErrorNetwork, Err: fmt.Errorf("Invalid matchedDN in packet"), Packet: packet}
226+
}
220227
return &Error{
221228
ResultCode: resultCode,
222229
MatchedDN: response.Children[1].Value.(string),

‎v3/error_test.go‎

Lines changed: 137 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package ldap
22

33
import (
4+
"bytes"
45
"errors"
56
"fmt"
67
"io"
@@ -90,27 +91,6 @@ func TestWrappedError(t *testing.T) {
9091
}
9192
}
9293

93-
// TestNilPacket tests that nil packets don't cause a panic.
94-
func TestNilPacket(t *testing.T) {
95-
// Test for nil packet
96-
err := GetLDAPError(nil)
97-
if !IsErrorWithCode(err, ErrorUnexpectedResponse) {
98-
t.Errorf("Should have an 'ErrorUnexpectedResponse' error in nil packets, got: %v", err)
99-
}
100-
101-
// Test for nil result
102-
kids := []*ber.Packet{
103-
{}, // Unused
104-
nil, // Can't be nil
105-
}
106-
pack := &ber.Packet{Children: kids}
107-
err = GetLDAPError(pack)
108-
109-
if !IsErrorWithCode(err, ErrorUnexpectedResponse) {
110-
t.Errorf("Should have an 'ErrorUnexpectedResponse' error in nil packets, got: %v", err)
111-
}
112-
}
113-
11494
// TestConnReadErr tests that an unexpected error reading from underlying
11595
// connection bubbles up to the goroutine which makes a request.
11696
func TestConnReadErr(t *testing.T) {
@@ -138,8 +118,16 @@ func TestConnReadErr(t *testing.T) {
138118
}
139119
}
140120

141-
// TestGetLDAPError tests parsing of result with a error response.
142-
func TestGetLDAPError(t *testing.T) {
121+
type testCorpusErrorEntry struct {
122+
packet *ber.Packet
123+
expectedResultCode uint16
124+
expectedMessage string
125+
shouldError bool
126+
}
127+
128+
func generateGetLDAPErrorCorpus() map[string]testCorpusErrorEntry {
129+
corpus := make(map[string]testCorpusErrorEntry)
130+
143131
diagnosticMessage := "Detailed error message"
144132
bindResponse := ber.Encode(ber.ClassApplication, ber.TypeConstructed, ApplicationBindResponse, nil, "Bind Response")
145133
bindResponse.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, int64(LDAPResultInvalidCredentials), "resultCode"))
@@ -148,39 +136,144 @@ func TestGetLDAPError(t *testing.T) {
148136
packet := ber.NewSequence("LDAPMessage")
149137
packet.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, int64(0), "messageID"))
150138
packet.AppendChild(bindResponse)
151-
err := GetLDAPError(packet)
152-
if err == nil {
153-
t.Errorf("Did not get error response")
139+
corpus["detailed errror message"] = testCorpusErrorEntry{
140+
packet: packet,
141+
expectedResultCode: LDAPResultInvalidCredentials,
142+
expectedMessage: diagnosticMessage,
143+
shouldError: true,
154144
}
155145

156-
ldapError := err.(*Error)
157-
if ldapError.ResultCode != LDAPResultInvalidCredentials {
158-
t.Errorf("Got incorrect error code in LDAP error; got %v, expected %v", ldapError.ResultCode, LDAPResultInvalidCredentials)
159-
}
160-
if ldapError.Err.Error() != diagnosticMessage {
161-
t.Errorf("Got incorrect error message in LDAP error; got %v, expected %v", ldapError.Err.Error(), diagnosticMessage)
146+
bindResponse = ber.Encode(ber.ClassApplication, ber.TypeConstructed, ApplicationBindResponse, nil, "Bind Response")
147+
bindResponse.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, int64(0), "resultCode"))
148+
bindResponse.AppendChild(ber.NewString(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, "", "matchedDN"))
149+
bindResponse.AppendChild(ber.NewString(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, "", "diagnosticMessage"))
150+
packet = ber.NewSequence("LDAPMessage")
151+
packet.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, int64(0), "messageID"))
152+
packet.AppendChild(bindResponse)
153+
corpus["no error"] = testCorpusErrorEntry{
154+
packet: packet,
155+
expectedResultCode: ErrorNetwork,
156+
expectedMessage: "",
162157
}
163-
}
164158

165-
// TestGetLDAPErrorInvalidResponse tests that responses with an unexpected ordering or combination of children
166-
// don't cause a panic.
167-
func TestGetLDAPErrorInvalidResponse(t *testing.T) {
168-
bindResponse := ber.Encode(ber.ClassApplication, ber.TypeConstructed, ApplicationBindResponse, nil, "Bind Response")
159+
// Test that responses with an unexpected ordering or combination of children
160+
// don't cause a panic.
161+
bindResponse = ber.Encode(ber.ClassApplication, ber.TypeConstructed, ApplicationBindResponse, nil, "Bind Response")
169162
bindResponse.AppendChild(ber.NewString(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, "dc=example,dc=org", "matchedDN"))
170163
bindResponse.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, int64(LDAPResultInvalidCredentials), "resultCode"))
171164
bindResponse.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, int64(LDAPResultInvalidCredentials), "resultCode"))
172-
packet := ber.NewSequence("LDAPMessage")
165+
packet = ber.NewSequence("LDAPMessage")
166+
packet.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, int64(0), "messageID"))
167+
packet.AppendChild(bindResponse)
168+
corpus["unexpected ordering"] = testCorpusErrorEntry{
169+
packet: packet,
170+
expectedResultCode: ErrorNetwork,
171+
expectedMessage: "Invalid packet format",
172+
shouldError: true,
173+
}
174+
175+
// Test that a nil ber Packet errors correctly and does not cause a panic.
176+
corpus["nil packet"] = testCorpusErrorEntry{
177+
packet: nil,
178+
expectedResultCode: ErrorUnexpectedResponse,
179+
expectedMessage: "Empty packet",
180+
shouldError: true,
181+
}
182+
183+
// Test that a nil first child errors correctly and does not cause a panic.
184+
kids := []*ber.Packet{
185+
{}, // Unused
186+
nil, // Can't be nil
187+
}
188+
packet = &ber.Packet{Children: kids}
189+
corpus["nil first child"] = testCorpusErrorEntry{
190+
packet: packet,
191+
expectedResultCode: ErrorUnexpectedResponse,
192+
expectedMessage: "Empty response in packet",
193+
shouldError: true,
194+
}
195+
196+
// Test that if the result code is nil, we get an appropriate error instead of a panic.
197+
// Panic message would be "interface conversion: interface {} is nil, not int64"
198+
diagnosticMessage = "Invalid result code in packet"
199+
bindResponse = ber.Encode(ber.ClassApplication, ber.TypeConstructed, ApplicationBindResponse, nil, "Bind Response")
200+
bindResponse.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, nil, "resultCode"))
201+
bindResponse.AppendChild(ber.NewString(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, "dc=example,dc=org", "matchedDN"))
202+
bindResponse.AppendChild(ber.NewString(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, diagnosticMessage, "diagnosticMessage"))
203+
packet = ber.NewSequence("LDAPMessage")
173204
packet.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, int64(0), "messageID"))
174205
packet.AppendChild(bindResponse)
175-
err := GetLDAPError(packet)
176-
if err == nil {
177-
t.Errorf("Did not get error response")
206+
corpus["nil result code"] = testCorpusErrorEntry{
207+
packet: packet,
208+
expectedResultCode: ErrorNetwork,
209+
expectedMessage: diagnosticMessage,
210+
shouldError: true,
178211
}
179212

180-
ldapError := err.(*Error)
181-
if ldapError.ResultCode != ErrorNetwork {
182-
t.Errorf("Got incorrect error code in LDAP error; got %v, expected %v", ldapError.ResultCode, ErrorNetwork)
213+
// Test that if the matchedDN is nil, we get an appropriate error instead of a panic.
214+
// Panic message would be "interface conversion: interface {} is nil, not string"
215+
panic_data := []byte("07A\x010\x7f\xff00\x02\x010D\"0000000000000000000000000000000000D\x010A\x010A\x010")
216+
packet, err := ber.ReadPacket(bytes.NewReader(panic_data))
217+
if err != nil {
218+
panic(fmt.Sprintf("failed to read packet for panic test: %s", err))
183219
}
220+
corpus["panic data"] = testCorpusErrorEntry{
221+
packet: packet,
222+
expectedResultCode: ErrorNetwork,
223+
expectedMessage: "Invalid matchedDN in packet",
224+
shouldError: true,
225+
}
226+
227+
return corpus
228+
}
229+
230+
func TestGetLDAPError(t *testing.T) {
231+
corpus := generateGetLDAPErrorCorpus()
232+
233+
for name, entry := range corpus {
234+
t.Run(name, func(t *testing.T) {
235+
err := GetLDAPError(entry.packet)
236+
237+
if !entry.shouldError {
238+
if err != nil {
239+
t.Errorf("Did not expect an error, but got: %v", err)
240+
}
241+
return
242+
} else if err == nil {
243+
t.Errorf("Expected an error response")
244+
return
245+
}
246+
247+
ldapError, ok := err.(*Error)
248+
if !ok {
249+
t.Fatalf("Expected error of type *Error, got %T", err)
250+
}
251+
252+
if ldapError.ResultCode != entry.expectedResultCode {
253+
t.Errorf("Got incorrect error code in LDAP error; got '%v', expected '%v'", ldapError.ResultCode, entry.expectedResultCode)
254+
}
255+
if ldapError.Err.Error() != entry.expectedMessage {
256+
t.Errorf("Got incorrect error message in LDAP error; got '%v', expected '%v'", ldapError.Err.Error(), entry.expectedMessage)
257+
}
258+
})
259+
}
260+
}
261+
262+
func FuzzGetLDAPError(f *testing.F) {
263+
corpus := generateGetLDAPErrorCorpus()
264+
for _, entry := range corpus {
265+
if entry.packet != nil {
266+
f.Add(entry.packet.ByteValue)
267+
}
268+
}
269+
270+
f.Fuzz(func(t *testing.T, data []byte) {
271+
packet, err := ber.ReadPacket(bytes.NewReader(data))
272+
if err != nil {
273+
return
274+
}
275+
_ = GetLDAPError(packet)
276+
})
184277
}
185278

186279
func TestErrorIs(t *testing.T) {
@@ -201,21 +294,6 @@ func TestErrorAs(t *testing.T) {
201294
}
202295
}
203296

204-
// TestGetLDAPErrorSuccess tests parsing of a result with no error (resultCode == 0).
205-
func TestGetLDAPErrorSuccess(t *testing.T) {
206-
bindResponse := ber.Encode(ber.ClassApplication, ber.TypeConstructed, ApplicationBindResponse, nil, "Bind Response")
207-
bindResponse.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, int64(0), "resultCode"))
208-
bindResponse.AppendChild(ber.NewString(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, "", "matchedDN"))
209-
bindResponse.AppendChild(ber.NewString(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, "", "diagnosticMessage"))
210-
packet := ber.NewSequence("LDAPMessage")
211-
packet.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, int64(0), "messageID"))
212-
packet.AppendChild(bindResponse)
213-
err := GetLDAPError(packet)
214-
if err != nil {
215-
t.Errorf("Successful responses should not produce an error, but got: %v", err)
216-
}
217-
}
218-
219297
// signalErrConn is a helpful type used with TestConnReadErr. It implements the
220298
// net.Conn interface to be used as a connection for the test. Most methods are
221299
// no-ops but the Read() method blocks until it receives a signal which it

‎v3/extended_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ func TestExtendedRequest_WhoAmI(t *testing.T) {
3838
func TestExtendedRequest_FastBind(t *testing.T) {
3939
conn, err := DialURL(ldapServer)
4040
if err != nil {
41-
t.Error(err)
41+
t.Fatal(err)
4242
}
4343
defer conn.Close()
4444

0 commit comments

Comments
 (0)