From 377e9dfc26948f71ad2abcb3396a7f18ac56a637 Mon Sep 17 00:00:00 2001 From: Tom Sellers Date: Thu, 19 Feb 2026 07:03:33 -0600 Subject: [PATCH 1/5] Address panic in GetLDAPError, add fuzzer --- Makefile | 2 +- v3/error.go | 4 + v3/error_test.go | 184 ++++++++++++++++++++++++++++++-------------- v3/extended_test.go | 2 +- 4 files changed, 131 insertions(+), 61 deletions(-) diff --git a/Makefile b/Makefile index 4d6ba1e9..d5e03c34 100644 --- a/Makefile +++ b/Makefile @@ -2,7 +2,7 @@ default: fmt vet lint build test -CONTAINER_CMD := $(shell command -v docker 2>/dev/null || shell command -v podman 2>/dev/null) +CONTAINER_CMD := $(shell (command -v docker 2>/dev/null || command -v podman 2>/dev/null)) ifeq ($(CONTAINER_CMD),) $(error Neither podman nor docker found in PATH) endif diff --git a/v3/error.go b/v3/error.go index 0014ffe2..78be4d3c 100644 --- a/v3/error.go +++ b/v3/error.go @@ -210,6 +210,10 @@ func GetLDAPError(packet *ber.Packet) error { } if response.ClassType == ber.ClassApplication && response.TagType == ber.TypeConstructed && len(response.Children) >= 3 { if ber.Type(response.Children[0].Tag) == ber.Type(ber.TagInteger) || ber.Type(response.Children[0].Tag) == ber.Type(ber.TagEnumerated) { + if response.Children[0].Value == nil { + return &Error{ResultCode: ErrorNetwork, Err: fmt.Errorf("Invalid result code in packet"), Packet: packet} + } + resultCode := uint16(response.Children[0].Value.(int64)) if resultCode == 0 { // No error return nil diff --git a/v3/error_test.go b/v3/error_test.go index 323f7665..e637f25c 100644 --- a/v3/error_test.go +++ b/v3/error_test.go @@ -1,6 +1,7 @@ package ldap import ( + "bytes" "errors" "fmt" "io" @@ -90,27 +91,6 @@ func TestWrappedError(t *testing.T) { } } -// TestNilPacket tests that nil packets don't cause a panic. -func TestNilPacket(t *testing.T) { - // Test for nil packet - err := GetLDAPError(nil) - if !IsErrorWithCode(err, ErrorUnexpectedResponse) { - t.Errorf("Should have an 'ErrorUnexpectedResponse' error in nil packets, got: %v", err) - } - - // Test for nil result - kids := []*ber.Packet{ - {}, // Unused - nil, // Can't be nil - } - pack := &ber.Packet{Children: kids} - err = GetLDAPError(pack) - - if !IsErrorWithCode(err, ErrorUnexpectedResponse) { - t.Errorf("Should have an 'ErrorUnexpectedResponse' error in nil packets, got: %v", err) - } -} - // TestConnReadErr tests that an unexpected error reading from underlying // connection bubbles up to the goroutine which makes a request. func TestConnReadErr(t *testing.T) { @@ -138,8 +118,17 @@ func TestConnReadErr(t *testing.T) { } } -// TestGetLDAPError tests parsing of result with a error response. -func TestGetLDAPError(t *testing.T) { +type testCorpusErrorEntry struct { + packet *ber.Packet + expectedError error + expectedResultCode uint16 + expectedMessage string + shouldError bool +} + +func generateGetLDAPErrorCorpus() map[string]testCorpusErrorEntry { + corpus := make(map[string]testCorpusErrorEntry) + diagnosticMessage := "Detailed error message" bindResponse := ber.Encode(ber.ClassApplication, ber.TypeConstructed, ApplicationBindResponse, nil, "Bind Response") bindResponse.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, int64(LDAPResultInvalidCredentials), "resultCode")) @@ -148,39 +137,131 @@ func TestGetLDAPError(t *testing.T) { packet := ber.NewSequence("LDAPMessage") packet.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, int64(0), "messageID")) packet.AppendChild(bindResponse) - err := GetLDAPError(packet) - if err == nil { - t.Errorf("Did not get error response") + corpus["detailed errror message"] = testCorpusErrorEntry{ + packet: packet, + expectedResultCode: LDAPResultInvalidCredentials, + expectedMessage: diagnosticMessage, + shouldError: true, } - ldapError := err.(*Error) - if ldapError.ResultCode != LDAPResultInvalidCredentials { - t.Errorf("Got incorrect error code in LDAP error; got %v, expected %v", ldapError.ResultCode, LDAPResultInvalidCredentials) - } - if ldapError.Err.Error() != diagnosticMessage { - t.Errorf("Got incorrect error message in LDAP error; got %v, expected %v", ldapError.Err.Error(), diagnosticMessage) + bindResponse = ber.Encode(ber.ClassApplication, ber.TypeConstructed, ApplicationBindResponse, nil, "Bind Response") + bindResponse.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, int64(0), "resultCode")) + bindResponse.AppendChild(ber.NewString(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, "", "matchedDN")) + bindResponse.AppendChild(ber.NewString(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, "", "diagnosticMessage")) + packet = ber.NewSequence("LDAPMessage") + packet.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, int64(0), "messageID")) + packet.AppendChild(bindResponse) + corpus["no error"] = testCorpusErrorEntry{ + packet: packet, + expectedResultCode: ErrorNetwork, + expectedMessage: "", } -} -// TestGetLDAPErrorInvalidResponse tests that responses with an unexpected ordering or combination of children -// don't cause a panic. -func TestGetLDAPErrorInvalidResponse(t *testing.T) { - bindResponse := ber.Encode(ber.ClassApplication, ber.TypeConstructed, ApplicationBindResponse, nil, "Bind Response") + // Test that responses with an unexpected ordering or combination of children + // don't cause a panic. + bindResponse = ber.Encode(ber.ClassApplication, ber.TypeConstructed, ApplicationBindResponse, nil, "Bind Response") bindResponse.AppendChild(ber.NewString(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, "dc=example,dc=org", "matchedDN")) bindResponse.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, int64(LDAPResultInvalidCredentials), "resultCode")) bindResponse.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, int64(LDAPResultInvalidCredentials), "resultCode")) - packet := ber.NewSequence("LDAPMessage") + packet = ber.NewSequence("LDAPMessage") packet.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, int64(0), "messageID")) packet.AppendChild(bindResponse) - err := GetLDAPError(packet) - if err == nil { - t.Errorf("Did not get error response") + corpus["unexpected ordering"] = testCorpusErrorEntry{ + packet: packet, + expectedResultCode: ErrorNetwork, + expectedMessage: "Invalid packet format", + shouldError: true, + } + + // Test that a nil ber Packet errors correctly and does not cause a panic. + corpus["nil packet"] = testCorpusErrorEntry{ + packet: nil, + expectedResultCode: ErrorUnexpectedResponse, + expectedMessage: "Empty packet", + shouldError: true, } - ldapError := err.(*Error) - if ldapError.ResultCode != ErrorNetwork { - t.Errorf("Got incorrect error code in LDAP error; got %v, expected %v", ldapError.ResultCode, ErrorNetwork) + // Test that a nil first child errors correctly and does not cause a panic. + kids := []*ber.Packet{ + {}, // Unused + nil, // Can't be nil + } + packet = &ber.Packet{Children: kids} + corpus["nil first child"] = testCorpusErrorEntry{ + packet: packet, + expectedResultCode: ErrorUnexpectedResponse, + expectedMessage: "Empty response in packet", + shouldError: true, } + + // Test that if the result code is nil, we get an appropriate error instead of a panic. + // Panic message would be "interface conversion: interface {} is nil, not int64" + diagnosticMessage = "Invalid result code in packet" + bindResponse = ber.Encode(ber.ClassApplication, ber.TypeConstructed, ApplicationBindResponse, nil, "Bind Response") + bindResponse.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, nil, "resultCode")) + bindResponse.AppendChild(ber.NewString(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, "dc=example,dc=org", "matchedDN")) + bindResponse.AppendChild(ber.NewString(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, diagnosticMessage, "diagnosticMessage")) + packet = ber.NewSequence("LDAPMessage") + packet.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, int64(0), "messageID")) + packet.AppendChild(bindResponse) + corpus["nil result code"] = testCorpusErrorEntry{ + packet: packet, + expectedResultCode: ErrorNetwork, + expectedMessage: diagnosticMessage, + shouldError: true, + } + + return corpus +} + +func TestGetLDAPError(t *testing.T) { + corpus := generateGetLDAPErrorCorpus() + + for name, entry := range corpus { + t.Run(name, func(t *testing.T) { + err := GetLDAPError(entry.packet) + + if !entry.shouldError { + if err != nil { + t.Errorf("Did not expect an error, but got: %v", err) + } + return + } + + if err == nil { + t.Errorf("Expected an error response") + } + + ldapError, ok := err.(*Error) + if !ok { + t.Fatalf("Expected error of type *Error, got %T", err) + } + + if ldapError.ResultCode != entry.expectedResultCode { + t.Errorf("Got incorrect error code in LDAP error; got %v, expected %v", ldapError.ResultCode, entry.expectedResultCode) + } + if ldapError.Err.Error() != entry.expectedMessage { + t.Errorf("Got incorrect error message in LDAP error; got %v, expected %v", ldapError.Err.Error(), entry.expectedMessage) + } + }) + } +} + +func FuzzGetLDAPError(f *testing.F) { + corpus := generateGetLDAPErrorCorpus() + for _, entry := range corpus { + if entry.packet != nil { + f.Add(entry.packet.ByteValue) + } + } + + f.Fuzz(func(t *testing.T, data []byte) { + packet, err := ber.ReadPacket(bytes.NewReader(data)) + if err != nil { + return + } + _ = GetLDAPError(packet) + }) } func TestErrorIs(t *testing.T) { @@ -201,21 +282,6 @@ func TestErrorAs(t *testing.T) { } } -// TestGetLDAPErrorSuccess tests parsing of a result with no error (resultCode == 0). -func TestGetLDAPErrorSuccess(t *testing.T) { - bindResponse := ber.Encode(ber.ClassApplication, ber.TypeConstructed, ApplicationBindResponse, nil, "Bind Response") - bindResponse.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, int64(0), "resultCode")) - bindResponse.AppendChild(ber.NewString(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, "", "matchedDN")) - bindResponse.AppendChild(ber.NewString(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, "", "diagnosticMessage")) - packet := ber.NewSequence("LDAPMessage") - packet.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, int64(0), "messageID")) - packet.AppendChild(bindResponse) - err := GetLDAPError(packet) - if err != nil { - t.Errorf("Successful responses should not produce an error, but got: %v", err) - } -} - // signalErrConn is a helpful type used with TestConnReadErr. It implements the // net.Conn interface to be used as a connection for the test. Most methods are // no-ops but the Read() method blocks until it receives a signal which it diff --git a/v3/extended_test.go b/v3/extended_test.go index 247e1dc5..8da26df4 100644 --- a/v3/extended_test.go +++ b/v3/extended_test.go @@ -38,7 +38,7 @@ func TestExtendedRequest_WhoAmI(t *testing.T) { func TestExtendedRequest_FastBind(t *testing.T) { conn, err := DialURL(ldapServer) if err != nil { - t.Error(err) + t.Fatal(err) } defer conn.Close() From c99a6afc428639e581eadc4e0dbb91651a175db6 Mon Sep 17 00:00:00 2001 From: Tom Sellers Date: Thu, 19 Feb 2026 07:13:30 -0600 Subject: [PATCH 2/5] adjust test logic --- v3/error_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/v3/error_test.go b/v3/error_test.go index e637f25c..55862e34 100644 --- a/v3/error_test.go +++ b/v3/error_test.go @@ -226,10 +226,9 @@ func TestGetLDAPError(t *testing.T) { t.Errorf("Did not expect an error, but got: %v", err) } return - } - - if err == nil { + } else if err == nil { t.Errorf("Expected an error response") + return } ldapError, ok := err.(*Error) From 55efac1484a82a6958ab11e0d7f5a7fc94aa041d Mon Sep 17 00:00:00 2001 From: Tom Sellers Date: Thu, 19 Feb 2026 07:20:49 -0600 Subject: [PATCH 3/5] remove unused struct field --- v3/error_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/v3/error_test.go b/v3/error_test.go index 55862e34..e7b63a58 100644 --- a/v3/error_test.go +++ b/v3/error_test.go @@ -120,7 +120,6 @@ func TestConnReadErr(t *testing.T) { type testCorpusErrorEntry struct { packet *ber.Packet - expectedError error expectedResultCode uint16 expectedMessage string shouldError bool From ab2aecb106e41f5af746669effddce1bafee0331 Mon Sep 17 00:00:00 2001 From: Tom Sellers Date: Thu, 19 Feb 2026 07:31:05 -0600 Subject: [PATCH 4/5] update make fuzz --- Makefile | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Makefile b/Makefile index d5e03c34..0feafdd3 100644 --- a/Makefile +++ b/Makefile @@ -53,9 +53,7 @@ test: go test -v -cover -race -count=1 . fuzz: - go test -fuzz=FuzzParseDN -fuzztime=600s . - go test -fuzz=FuzzDecodeEscapedSymbols -fuzztime=600s . - go test -fuzz=FuzzEscapeDN -fuzztime=600s . + (cd v3 && go test -fuzz=FuzzGetLDAPError -fuzztime=600s .) # Capture output and force failure when there is non-empty output fmt: From b917f492299920cb642fc07a2c3fce95c5a7f937 Mon Sep 17 00:00:00 2001 From: Tom Sellers Date: Thu, 19 Feb 2026 11:20:52 -0600 Subject: [PATCH 5/5] Add coverage for nil matchDN in packet passed to GetLDAPError --- v3/error.go | 3 +++ v3/error_test.go | 18 ++++++++++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/v3/error.go b/v3/error.go index 78be4d3c..1cf09c4f 100644 --- a/v3/error.go +++ b/v3/error.go @@ -221,6 +221,9 @@ func GetLDAPError(packet *ber.Packet) error { if ber.Type(response.Children[1].Tag) == ber.Type(ber.TagOctetString) && ber.Type(response.Children[2].Tag) == ber.Type(ber.TagOctetString) { + if response.Children[1].Value == nil { + return &Error{ResultCode: ErrorNetwork, Err: fmt.Errorf("Invalid matchedDN in packet"), Packet: packet} + } return &Error{ ResultCode: resultCode, MatchedDN: response.Children[1].Value.(string), diff --git a/v3/error_test.go b/v3/error_test.go index e7b63a58..fd8fd5c4 100644 --- a/v3/error_test.go +++ b/v3/error_test.go @@ -210,6 +210,20 @@ func generateGetLDAPErrorCorpus() map[string]testCorpusErrorEntry { shouldError: true, } + // Test that if the matchedDN is nil, we get an appropriate error instead of a panic. + // Panic message would be "interface conversion: interface {} is nil, not string" + panic_data := []byte("07A\x010\x7f\xff00\x02\x010D\"0000000000000000000000000000000000D\x010A\x010A\x010") + packet, err := ber.ReadPacket(bytes.NewReader(panic_data)) + if err != nil { + panic(fmt.Sprintf("failed to read packet for panic test: %s", err)) + } + corpus["panic data"] = testCorpusErrorEntry{ + packet: packet, + expectedResultCode: ErrorNetwork, + expectedMessage: "Invalid matchedDN in packet", + shouldError: true, + } + return corpus } @@ -236,10 +250,10 @@ func TestGetLDAPError(t *testing.T) { } if ldapError.ResultCode != entry.expectedResultCode { - t.Errorf("Got incorrect error code in LDAP error; got %v, expected %v", ldapError.ResultCode, entry.expectedResultCode) + t.Errorf("Got incorrect error code in LDAP error; got '%v', expected '%v'", ldapError.ResultCode, entry.expectedResultCode) } if ldapError.Err.Error() != entry.expectedMessage { - t.Errorf("Got incorrect error message in LDAP error; got %v, expected %v", ldapError.Err.Error(), entry.expectedMessage) + t.Errorf("Got incorrect error message in LDAP error; got '%v', expected '%v'", ldapError.Err.Error(), entry.expectedMessage) } }) }