Skip to content

Address panic in GetLDAPError, add fuzzer #582

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cpuschma merged 5 commits into from
Mar 1, 2026
Merged

Address panic in GetLDAPError, add fuzzer #582

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
377e9df
Address panic in GetLDAPError, add fuzzer
TomSellers Feb 19, 2026
c99a6af
adjust test logic
TomSellers Feb 19, 2026
55efac1
remove unused struct field
TomSellers Feb 19, 2026
ab2aecb
update make fuzz
TomSellers Feb 19, 2026
b917f49
Add coverage for nil matchDN in packet passed to GetLDAPError
TomSellers Feb 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Copy link
Copy Markdown
Contributor Author

@TomSellers TomSellers Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the prior code, the second shell was not being treated as a make keyword. This resulted in the podman check not working. The change here is to group the shell statements and pass them to shell.

ifeq ($(CONTAINER_CMD),)
$(error Neither podman nor docker found in PATH)
endif
Expand Down Expand Up @@ -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:
Expand Down
7 changes: 7 additions & 0 deletions v3/error.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,13 +210,20 @@ 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
}

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),
Expand Down
196 changes: 137 additions & 59 deletions v3/error_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ldap

import (
"bytes"
"errors"
"fmt"
"io"
Expand Down Expand Up @@ -90,27 +91,6 @@ func TestWrappedError(t *testing.T) {
}
}

// TestNilPacket tests that nil packets don't cause a panic.
func TestNilPacket(t *testing.T) {
Copy link
Copy Markdown
Contributor Author

@TomSellers TomSellers Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and elsewhere below: All removed tests have been added to generateGetLDAPErrorCorpus.

// 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) {
Expand Down Expand Up @@ -138,8 +118,16 @@ 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
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"))
Expand All @@ -148,39 +136,144 @@ 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)
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,
}

// 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)
err := GetLDAPError(packet)
if err == nil {
t.Errorf("Did not get error response")
corpus["nil result code"] = testCorpusErrorEntry{
packet: packet,
expectedResultCode: ErrorNetwork,
expectedMessage: diagnosticMessage,
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 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
}

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
} else if err == nil {
t.Errorf("Expected an error response")
return
}

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) {
Expand All @@ -201,21 +294,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
Expand Down
2 changes: 1 addition & 1 deletion v3/extended_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Copy Markdown
Contributor Author

@TomSellers TomSellers Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This now matches the logic in TestExtendedRequest_WhoAmI above and ensures that the test doesn't panic due to calling conn.Close() on a nil conn if the test LDAP server isn't available.

}
defer conn.Close()

Expand Down
Loading