reject a channel parent that is its own descendant

handleChannelState applied whatever parent the server named, so a channel
could be made its own ancestor. That leaves Parent/Children cyclic, and
everything that walks the channel tree afterwards recurses until it exhausts
memory.

Ignore such a move and keep the existing parent rather than corrupting the
graph. The ancestry walk is itself bounded so a graph that is already cyclic
cannot hang the check.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

gumble/gumble/channel_cycle_regression_test.go

@@ -0,0 +1,84 @@
+package gumble
+
+import (
+	"testing"
+
+	"git.stormux.org/storm/barnard/gumble/gumble/MumbleProto"
+	"google.golang.org/protobuf/proto"
+)
+
+// Regression: handleChannelState accepted any parent the server named, so a
+// channel could be made its own ancestor. Everything that walks the resulting
+// Parent/Children graph then recurses until it exhausts memory.
+func TestChannelStateRejectsSelfParent(t *testing.T) {
+	c := &Client{Config: NewConfig(), Channels: make(Channels)}
+	root := c.Channels.create(0)
+	child := c.Channels.create(1)
+	child.Parent = root
+	root.Children[child.ID] = child
+
+	id, parent := child.ID, child.ID
+	data, _ := proto.Marshal(&MumbleProto.ChannelState{ChannelId: &id, Parent: &parent})
+	if err := c.handleChannelState(data); err != nil {
+		t.Fatal(err)
+	}
+
+	if child.Parent == child {
+		t.Fatal("channel was made its own parent")
+	}
+	if _, ok := child.Children[child.ID]; ok {
+		t.Fatal("channel was made its own child")
+	}
+	if child.Parent != root {
+		t.Fatal("the rejected move should have left the original parent intact")
+	}
+}
+
+// A channel must not be reparented under one of its own descendants either.
+func TestChannelStateRejectsDescendantParent(t *testing.T) {
+	c := &Client{Config: NewConfig(), Channels: make(Channels)}
+	root := c.Channels.create(0)
+	middle := c.Channels.create(1)
+	leaf := c.Channels.create(2)
+	middle.Parent, root.Children[middle.ID] = root, middle
+	leaf.Parent, middle.Children[leaf.ID] = middle, leaf
+
+	id, parent := middle.ID, leaf.ID
+	data, _ := proto.Marshal(&MumbleProto.ChannelState{ChannelId: &id, Parent: &parent})
+	if err := c.handleChannelState(data); err != nil {
+		t.Fatal(err)
+	}
+
+	if middle.Parent != root {
+		t.Fatal("a cyclic reparent was applied instead of ignored")
+	}
+	if isChannelDescendant(middle.Parent, middle) {
+		t.Fatal("channel graph is cyclic")
+	}
+}
+
+// A legitimate move must still be applied.
+func TestChannelStateAllowsNonCyclicMove(t *testing.T) {
+	c := &Client{Config: NewConfig(), Channels: make(Channels)}
+	root := c.Channels.create(0)
+	a := c.Channels.create(1)
+	b := c.Channels.create(2)
+	a.Parent, root.Children[a.ID] = root, a
+	b.Parent, root.Children[b.ID] = root, b
+
+	id, parent := b.ID, a.ID
+	data, _ := proto.Marshal(&MumbleProto.ChannelState{ChannelId: &id, Parent: &parent})
+	if err := c.handleChannelState(data); err != nil {
+		t.Fatal(err)
+	}
+
+	if b.Parent != a {
+		t.Fatal("a valid reparent was rejected")
+	}
+	if a.Children[b.ID] != b {
+		t.Fatal("child link missing after a valid reparent")
+	}
+	if _, ok := root.Children[b.ID]; ok {
+		t.Fatal("stale child link left on the old parent")
+	}
+}

gumble/gumble/handlers.go

@@ -447,6 +447,23 @@ func (c *Client) handleChannelRemove(buffer []byte) error {
 	return nil
 }
 
+// maxChannelDepth bounds ancestry walks over a channel tree that may already
+// be cyclic. Real Mumble trees are far shallower than this.
+const maxChannelDepth = 1024
+
+// isChannelDescendant reports whether candidate is channel itself or sits
+// below it in the channel tree. The walk is bounded so an already-cyclic
+// graph cannot hang the caller.
+func isChannelDescendant(candidate, channel *Channel) bool {
+	for i := 0; candidate != nil && i <= maxChannelDepth; i++ {
+		if candidate == channel {
+			return true
+		}
+		candidate = candidate.Parent
+	}
+	return false
+}
+
 func (c *Client) handleChannelState(buffer []byte) error {
 	var packet MumbleProto.ChannelState
 	if err := proto.Unmarshal(buffer, &packet); err != nil {
@@ -473,10 +490,18 @@ func (c *Client) handleChannelState(buffer []byte) error {
 		}
 		event.Channel = channel
 		if packet.Parent != nil {
+			newParent := c.Channels[*packet.Parent]
+			// Reparenting a channel under itself or one of its own
+			// descendants makes Parent/Children cyclic, and anything that
+			// walks the tree then recurses until it exhausts memory. Ignore
+			// the move rather than corrupt the channel graph.
+			if isChannelDescendant(newParent, channel) {
+				log.Warn("handleChannelState: ignoring cyclic parent %d for channel %d",
+					*packet.Parent, channelID)
+			} else {
 				if channel.Parent != nil {
 					delete(channel.Parent.Children, channelID)
 				}
-			newParent := c.Channels[*packet.Parent]
 				if newParent != channel.Parent {
 					event.Type |= ChannelChangeMoved
 				}
@@ -485,6 +510,7 @@ func (c *Client) handleChannelState(buffer []byte) error {
 					channel.Parent.Children[channel.ID] = channel
 				}
 			}
+		}
 		if packet.Name != nil {
 			if *packet.Name != channel.Name {
 				event.Type |= ChannelChangeName