Harden LDAP control decoding against malformed BER input

control.go

@@ -100,8 +100,24 @@ func FindControl(controls []Control, controlType string) Control {
 	return nil
 }
 
-func DecodeControl(packet *ber.Packet) Control {
-	ControlType := packet.Children[0].Value.(string)
+func DecodeControl(packet *ber.Packet) (control Control, err error) {
+	defer func() {
+		if r := recover(); r != nil {
+			control = nil
+			err = fmt.Errorf("ldap: failed to decode control: %v", r)
+		}
+	}()
+
+	if packet == nil || len(packet.Children) == 0 {
+		return nil, fmt.Errorf("ldap: failed to decode control: malformed control packet")
+	}
+	if packet.Children[0] == nil {
+		return nil, fmt.Errorf("ldap: failed to decode control: malformed control packet")
+	}
+	ControlType, ok := packet.Children[0].Value.(string)
+	if !ok {
+		return nil, fmt.Errorf("ldap: failed to decode control: malformed control type")
+	}
 	packet.Children[0].Description = "Control Type (" + ControlTypeMap[ControlType] + ")"
 	c := new(ControlString)
 	c.ControlType = ControlType
@@ -111,8 +127,18 @@ func DecodeControl(packet *ber.Packet) Control {
 		value := packet.Children[1]
 		if len(packet.Children) == 3 {
 			value = packet.Children[2]
+			if packet.Children[1] == nil {
+				return nil, fmt.Errorf("ldap: failed to decode control: malformed control criticality")
+			}
 			packet.Children[1].Description = "Criticality"
-			c.Criticality = packet.Children[1].Value.(bool)
+			criticality, ok := packet.Children[1].Value.(bool)
+			if !ok {
+				return nil, fmt.Errorf("ldap: failed to decode control: malformed control criticality")
+			}
+			c.Criticality = criticality
+		}
+		if value == nil {
+			return nil, fmt.Errorf("ldap: failed to decode control: malformed control value")
 		}
 
 		value.Description = "Control Value"
@@ -126,18 +152,33 @@ func DecodeControl(packet *ber.Packet) Control {
 				value.Value = nil
 				value.AppendChild(valueChildren)
 			}
+			// Exactly one controlValue, RFC2696
+			if len(value.Children) != 1 || value.Children[0] == nil {
+				return nil, fmt.Errorf("ldap: failed to decode control: malformed paging control value")
+			}
 			value = value.Children[0]
 			value.Description = "Search Control Value"
+			if len(value.Children) < 2 || value.Children[0] == nil || value.Children[1] == nil {
+				return nil, fmt.Errorf("ldap: failed to decode control: malformed paging control value")
+			}
 			value.Children[0].Description = "Paging Size"
 			value.Children[1].Description = "Cookie"
-			c.PagingSize = uint32(value.Children[0].Value.(int64))
+			pagingSize, ok := value.Children[0].Value.(int64)
+			if !ok || pagingSize < 0 || pagingSize > int64(^uint32(0)) {
+				return nil, fmt.Errorf("ldap: failed to decode control: malformed paging control size")
+			}
+			c.PagingSize = uint32(pagingSize)
 			c.Cookie = value.Children[1].Data.Bytes()
 			value.Children[1].Value = c.Cookie
-			return c
+			return c, nil
+		}
+		controlValue, ok := value.Value.(string)
+		if !ok {
+			return nil, fmt.Errorf("ldap: failed to decode control: malformed control value")
 		}
-		c.ControlValue = value.Value.(string)
+		c.ControlValue = controlValue
 	}
-	return c
+	return c, nil
 }
 
 func NewControlString(controlType string, criticality bool, controlValue string) *ControlString {

protocol_test.go

@@ -0,0 +1,184 @@
+package ldap
+
+import (
+	"bytes"
+	"testing"
+
+	ber "github.com/go-asn1-ber/asn1-ber"
+)
+
+// redecode serializes a packet and parses it again. DecodeControl runs on
+// controls parsed from the wire, not on in-memory packets, and the original
+// unchecked type assertions / child indexing only panic on wire-decoded
+// input. Round-tripping through bytes reproduces that path faithfully.
+func redecode(p *ber.Packet) *ber.Packet {
+	if p == nil {
+		return nil
+	}
+	return ber.DecodePacket(p.Bytes())
+}
+
+func controlSeq(children ...*ber.Packet) *ber.Packet {
+	seq := ber.Encode(ber.ClassUniversal, ber.TypeConstructed, ber.TagSequence, nil, "Control")
+	for _, c := range children {
+		seq.AppendChild(c)
+	}
+	return seq
+}
+
+func octet(s string) *ber.Packet {
+	return ber.NewString(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, s, "")
+}
+
+// pagingControl builds a paging control whose value wraps the supplied inner
+// sequence, mirroring ControlPaging.Encode() so the byte layout matches the
+// real wire format.
+func pagingControl(inner *ber.Packet) *ber.Packet {
+	ctrl := ber.Encode(ber.ClassUniversal, ber.TypeConstructed, ber.TagSequence, nil, "Control")
+	ctrl.AppendChild(octet(ControlTypePaging))
+	value := ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, nil, "Control Value (Paging)")
+	value.AppendChild(inner)
+	ctrl.AppendChild(value)
+	return ctrl
+}
+
+func searchValueSeq(children ...*ber.Packet) *ber.Packet {
+	seq := ber.Encode(ber.ClassUniversal, ber.TypeConstructed, ber.TagSequence, nil, "Search Control Value")
+	for _, c := range children {
+		seq.AppendChild(c)
+	}
+	return seq
+}
+
+func integer(v uint64) *ber.Packet {
+	return ber.NewInteger(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, v, "")
+}
+
+func TestDecodeControl(t *testing.T) {
+	tests := []struct {
+		name    string
+		packet  *ber.Packet
+		wantErr bool
+		check   func(t *testing.T, c Control)
+	}{
+		{
+			name:   "string control, type only",
+			packet: redecode((&ControlString{ControlType: "1.2.3.4"}).Encode()),
+			check: func(t *testing.T, c Control) {
+				cs, ok := c.(*ControlString)
+				if !ok {
+					t.Fatalf("got %T, want *ControlString", c)
+				}
+				if cs.ControlType != "1.2.3.4" {
+					t.Errorf("ControlType = %q, want 1.2.3.4", cs.ControlType)
+				}
+				if cs.Criticality {
+					t.Errorf("Criticality = true, want false")
+				}
+				if cs.ControlValue != "" {
+					t.Errorf("ControlValue = %q, want empty", cs.ControlValue)
+				}
+			},
+		},
+		{
+			name:   "string control, type and value",
+			packet: redecode((&ControlString{ControlType: "1.2.3.4", ControlValue: "payload"}).Encode()),
+			check: func(t *testing.T, c Control) {
+				cs := c.(*ControlString)
+				if cs.Criticality {
+					t.Errorf("Criticality = true, want false")
+				}
+				if cs.ControlValue != "payload" {
+					t.Errorf("ControlValue = %q, want payload", cs.ControlValue)
+				}
+			},
+		},
+		{
+			name:   "string control, type criticality and value",
+			packet: redecode((&ControlString{ControlType: "1.2.3.4", Criticality: true, ControlValue: "payload"}).Encode()),
+			check: func(t *testing.T, c Control) {
+				cs := c.(*ControlString)
+				if !cs.Criticality {
+					t.Errorf("Criticality = false, want true")
+				}
+				if cs.ControlValue != "payload" {
+					t.Errorf("ControlValue = %q, want payload", cs.ControlValue)
+				}
+			},
+		},
+		{
+			name:   "paging control round-trip",
+			packet: redecode((&ControlPaging{PagingSize: 100, Cookie: []byte("cookie")}).Encode()),
+			check: func(t *testing.T, c Control) {
+				cp, ok := c.(*ControlPaging)
+				if !ok {
+					t.Fatalf("got %T, want *ControlPaging", c)
+				}
+				if cp.PagingSize != 100 {
+					t.Errorf("PagingSize = %d, want 100", cp.PagingSize)
+				}
+				if !bytes.Equal(cp.Cookie, []byte("cookie")) {
+					t.Errorf("Cookie = %q, want cookie", cp.Cookie)
+				}
+			},
+		},
+		{
+			name:    "nil packet",
+			packet:  nil,
+			wantErr: true,
+		},
+		{
+			name:    "empty control sequence",
+			packet:  redecode(controlSeq()),
+			wantErr: true,
+		},
+		{
+			name:    "control type not an octet string",
+			packet:  redecode(controlSeq(integer(5))),
+			wantErr: true,
+		},
+		{
+			name: "criticality not a boolean",
+			packet: redecode(controlSeq(
+				octet("1.2.3.4"),
+				octet("not-a-bool"),
+				octet("value"),
+			)),
+			wantErr: true,
+		},
+		{
+			name:    "paging value sequence too short",
+			packet:  redecode(pagingControl(searchValueSeq(integer(10)))),
+			wantErr: true,
+		},
+		{
+			name:    "paging size exceeds uint32",
+			packet:  redecode(pagingControl(searchValueSeq(integer(uint64(1)<<32), octet("ck")))),
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c, err := DecodeControl(tt.packet)
+			if tt.wantErr {
+				if err == nil {
+					t.Fatalf("DecodeControl() error = nil, want error")
+				}
+				if c != nil {
+					t.Errorf("DecodeControl() control = %v, want nil on error", c)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("DecodeControl() unexpected error: %v", err)
+			}
+			if c == nil {
+				t.Fatalf("DecodeControl() control = nil, want non-nil")
+			}
+			if tt.check != nil {
+				tt.check(t, c)
+			}
+		})
+	}
+}

search.go

@@ -331,7 +331,11 @@ func (l *Conn) Search(searchRequest *SearchRequest) (*SearchResult, error) {
 			}
 			if len(packet.Children) == 3 {
 				for _, child := range packet.Children[2].Children {
-					result.Controls = append(result.Controls, DecodeControl(child))
+					control, err := DecodeControl(child)
+					if err != nil {
+						return result, NewError(ErrorNetwork, err)
+					}
+					result.Controls = append(result.Controls, control)
 				}
 			}
 			foundSearchResultDone = true

server.go

@@ -222,6 +222,13 @@ func (server *Server) Close() {
 }
 
 func (server *Server) handleConnection(conn net.Conn) {
+	defer func() {
+		if r := recover(); r != nil {
+			log.Printf("ldap: (THIS SHOULD NEVER HAPPEN, PLEASE REPORT) panic in handler from %s: %v", conn.RemoteAddr(), r)
+		}
+		conn.Close()
+	}()
+
 	boundDN := "" // "" == anonymous
 
 handler:
@@ -256,7 +263,18 @@ handler:
 		controls := []Control{}
 		if len(packet.Children) > 2 {
 			for _, child := range packet.Children[2].Children {
-				controls = append(controls, DecodeControl(child))
+				control, err := DecodeControl(child)
+				if err != nil {
+					log.Printf("DecodeControl error %s", err.Error())
+					responsePacket := encodeProtocolErrorResponse(messageID, req.Tag)
+					if responsePacket != nil {
+						if err = sendPacket(conn, responsePacket); err != nil {
+							log.Printf("sendPacket error %s", err.Error())
+						}
+					}
+					break handler
+				}
+				controls = append(controls, control)
 			}
 		}
 
@@ -401,6 +419,29 @@ func sendPacket(conn net.Conn, packet *ber.Packet) error {
 	return nil
 }
 
+func encodeProtocolErrorResponse(messageID uint64, requestType ber.Tag) *ber.Packet {
+	switch requestType {
+	case ApplicationBindRequest:
+		return encodeBindResponse(messageID, LDAPResultProtocolError)
+	case ApplicationSearchRequest:
+		return encodeSearchDone(messageID, LDAPResultProtocolError)
+	case ApplicationModifyRequest:
+		return encodeLDAPResponse(messageID, ApplicationModifyResponse, LDAPResultProtocolError, LDAPResultCodeMap[LDAPResultProtocolError])
+	case ApplicationAddRequest:
+		return encodeLDAPResponse(messageID, ApplicationAddResponse, LDAPResultProtocolError, LDAPResultCodeMap[LDAPResultProtocolError])
+	case ApplicationDelRequest:
+		return encodeLDAPResponse(messageID, ApplicationDelResponse, LDAPResultProtocolError, LDAPResultCodeMap[LDAPResultProtocolError])
+	case ApplicationModifyDNRequest:
+		return encodeLDAPResponse(messageID, ApplicationModifyDNResponse, LDAPResultProtocolError, LDAPResultCodeMap[LDAPResultProtocolError])
+	case ApplicationCompareRequest:
+		return encodeLDAPResponse(messageID, ApplicationCompareResponse, LDAPResultProtocolError, LDAPResultCodeMap[LDAPResultProtocolError])
+	case ApplicationExtendedRequest:
+		return encodeLDAPResponse(messageID, ApplicationExtendedResponse, LDAPResultProtocolError, LDAPResultCodeMap[LDAPResultProtocolError])
+	default:
+		return nil
+	}
+}
+
 func routeFunc(dn string, funcNames []string) string {
 	bestPick := ""
 	bestPickWeight := 0