v3/control: replace unchecked type asserts in DecodeControl with comma-ok

DecodeControl trusts that every controlType is a string and every
criticality is a bool, and falls back to value.Value.(string) for any
unknown control type. When a malformed or hostile server returns a
non-string (or nil) in any of these positions, the .(string) /
.(bool) cast panics inside the caller's goroutine. The user repro in
#561 hit the default case where value.Value was nil:

	panic: interface conversion: interface {} is nil, not string
	  go-ldap/ldap/v3.DecodeControl(...)
	  go-ldap/ldap/v3.(*Conn).SimpleBind(...)

Use comma-ok asserts at every control-frame field, return clear
errors when the cast fails, and in the default ControlString case
fall back to value.Data.String() when value.Value isn't a string so
the application gets the bytes instead of a crash.

The existing TestDecodeControl table covers the normal-path cases
and still passes; the new behaviour only kicks in on malformed
input where the previous code panicked.

Closes #561

Signed-off-by: Charlie Tonneslan <cst0520@gmail.com>

Author: c-tonneslan

v3/control.go

@@ -565,12 +565,20 @@ func DecodeControl(packet *ber.Packet) (Control, error) {
 	case 1:
 		// just type, no criticality or value
 		packet.Children[0].Description = "Control Type (" + ControlTypeMap[ControlType] + ")"
-		ControlType = packet.Children[0].Value.(string)
+		ct, ok := packet.Children[0].Value.(string)
+		if !ok {
+			return nil, fmt.Errorf("control type is not a string: %T", packet.Children[0].Value)
+		}
+		ControlType = ct
 
 	case 2:
 		packet.Children[0].Description = "Control Type (" + ControlTypeMap[ControlType] + ")"
 		if packet.Children[0].Value != nil {
-			ControlType = packet.Children[0].Value.(string)
+			ct, ok := packet.Children[0].Value.(string)
+			if !ok {
+				return nil, fmt.Errorf("control type is not a string: %T", packet.Children[0].Value)
+			}
+			ControlType = ct
 		} else if packet.Children[0].Data != nil {
 			ControlType = packet.Children[0].Data.String()
 		} else {
@@ -579,20 +587,28 @@ func DecodeControl(packet *ber.Packet) (Control, error) {
 
 		// Children[1] could be criticality or value (both are optional)
 		// duck-type on whether this is a boolean
-		if _, ok := packet.Children[1].Value.(bool); ok {
+		if crit, ok := packet.Children[1].Value.(bool); ok {
 			packet.Children[1].Description = "Criticality"
-			Criticality = packet.Children[1].Value.(bool)
+			Criticality = crit
 		} else {
 			packet.Children[1].Description = "Control Value"
 			value = packet.Children[1]
 		}
 
 	case 3:
 		packet.Children[0].Description = "Control Type (" + ControlTypeMap[ControlType] + ")"
-		ControlType = packet.Children[0].Value.(string)
+		ct, ok := packet.Children[0].Value.(string)
+		if !ok {
+			return nil, fmt.Errorf("control type is not a string: %T", packet.Children[0].Value)
+		}
+		ControlType = ct
 
 		packet.Children[1].Description = "Criticality"
-		Criticality = packet.Children[1].Value.(bool)
+		crit, ok := packet.Children[1].Value.(bool)
+		if !ok {
+			return nil, fmt.Errorf("criticality is not a bool: %T", packet.Children[1].Value)
+		}
+		Criticality = crit
 
 		packet.Children[2].Description = "Control Value"
 		value = packet.Children[2]
@@ -729,7 +745,16 @@ func DecodeControl(packet *ber.Packet) (Control, error) {
 		c.ControlType = ControlType
 		c.Criticality = Criticality
 		if value != nil {
-			c.ControlValue = value.Value.(string)
+			// A non-conforming or malicious server can send a non-string
+			// (or nil) value here; the previous unchecked cast panicked
+			// the calling goroutine, see #561. Fall back to the raw bytes
+			// when the value isn't a string so we surface an error
+			// instead of crashing.
+			if s, ok := value.Value.(string); ok {
+				c.ControlValue = s
+			} else if value.Data != nil {
+				c.ControlValue = value.Data.String()
+			}
 		}
 		return c, nil
 	}