Address panic in GetLDAPError, add fuzzer

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

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

v3/error_test.go

@@ -1,6 +1,7 @@
 package ldap
 
 import (
+	"bytes"
 	"errors"
 	"fmt"
 	"io"
@@ -90,27 +91,6 @@
 	}
 }
 
-// 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 @@
 	}
 }
 
-// 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,130 @@
 	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,
 	}
 
-	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 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)
+	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
+			} 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) {
@@ -201,21 +281,6 @@
 	}
 }
 
-// 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

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()