Reject malformed ACL and user-list records

fix.txt

@@ -161,7 +161,7 @@ Priority 2: protocol and data correctness
     not remove channel from the Links maps of old peers. Remove reciprocal old
     links before replacement and add link add/remove/full-replacement tests.
 
-20. Malformed protobuf fields can panic handlers
+[x] 20. Malformed protobuf fields can panic handlers
     File: gumble/gumble/handlers.go
     Several optional proto fields are dereferenced without validation, notably
     ACL group.Name and UserList_User.UserId. Validate required fields before

gumble/gumble/acl_regression_test.go

@@ -0,0 +1,24 @@
+package gumble
+
+import (
+	"testing"
+
+	"git.stormux.org/storm/barnard/gumble/gumble/MumbleProto"
+	"google.golang.org/protobuf/proto"
+)
+
+// Regression: an ACL group without its optional name dereferenced nil in the
+// TCP handler, allowing malformed server data to crash the client.
+func TestACLRejectsGroupWithoutName(t *testing.T) {
+	id := uint32(1)
+	packet := &MumbleProto.ACL{ChannelId: &id, Groups: []*MumbleProto.ACL_ChanGroup{{}}}
+	data, err := proto.MarshalOptions{AllowPartial: true}.Marshal(packet)
+	if err != nil {
+		t.Fatal(err)
+	}
+	c := &Client{Config: NewConfig(), Channels: make(Channels)}
+	c.Channels.create(id)
+	if err := c.handleACL(data); err == nil {
+		t.Fatal("accepted malformed ACL group")
+	}
+}

gumble/gumble/handlers.go

@@ -899,6 +899,9 @@ func (c *Client) handleACL(buffer []byte) error {
 	if packet.Groups != nil {
 		acl.Groups = make([]*ACLGroup, 0, len(packet.Groups))
 		for _, group := range packet.Groups {
+			if group == nil || group.Name == nil {
+				return errInvalidProtobuf
+			}
 			aclGroup := &ACLGroup{
 				Name:         *group.Name,
 				Inherited:    group.GetInherited(),
@@ -1079,6 +1082,9 @@ func (c *Client) handleUserList(buffer []byte) error {
 	}
 
 	for _, user := range packet.Users {
+		if user == nil || user.UserId == nil {
+			return errInvalidProtobuf
+		}
 		registeredUser := &RegisteredUser{
 			UserID: *user.UserId,
 		}