Skip to content

Fix malformed batch queue growth vulnerability in Unbatcher #4113

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
miwarnec merged 4 commits into from
Jun 17, 2026
+203 −66
Merged

Fix malformed batch queue growth vulnerability in Unbatcher #4113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dba0f64
fix: handle malformed batches in Unbatcher to prevent queue growth at…
Copilot Jun 16, 2026
d9c64f7
refactor: use Array.Empty<byte>() instead of new byte[0] in Unbatcher…
Copilot Jun 16, 2026
13249a1
fix: use explicit ArraySegment<byte> in Unbatcher.Clear for cross-ver…
Copilot Jun 17, 2026
028c43c
Use #if UNITY_2021_3_OR_NEWER for implicit ArraySegment conversion in…
Copilot Jun 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 24 additions & 2 deletions Assets/Mirror/Core/Batching/Unbatcher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,23 @@ public class Unbatcher

public int BatchesCount => batches.Count;

// clear all batches and return writers to pool.
// called when a malformed batch is detected or connection disconnects.
public void Clear()
{
while (batches.Count > 0)
{
NetworkWriterPooled writer = batches.Dequeue();
NetworkWriterPool.Return(writer);
}
// reset reader so it doesn't point to returned batch
#if UNITY_2021_3_OR_NEWER
reader.SetBuffer(Array.Empty<byte>());
#else
reader.SetBuffer(new ArraySegment<byte>(Array.Empty<byte>()));
#endif
}

// NetworkReader is only created once,
// then pointed to the first batch.
readonly NetworkReader reader = new NetworkReader(new byte[0]);
Expand Down Expand Up @@ -117,9 +134,14 @@ public bool GetNextMessage(out ArraySegment<byte> message, out double remoteTime
// see Batcher.AddMessage comments for explanation.
int size = (int)Compression.DecompressVarUInt(reader);

// validate size prefix, in case attackers send malicious data
// validate size prefix, in case attackers send malicious data.
// a size larger than remaining bytes means the batch is malformed.
// clear all batches and throw so the caller can disconnect.
if (reader.Remaining < size)
return false;
{
Clear();
throw new InvalidOperationException($"GetNextMessage: malformed batch with message size {size} > remaining {reader.Remaining}. All batches cleared.");
}

// return the message of size
message = reader.ReadBytesSegment(size);
Expand Down
83 changes: 51 additions & 32 deletions Assets/Mirror/Core/NetworkClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,53 +354,68 @@ internal static void OnTransportData(ArraySegment<byte> data, int channelId)
// would only be processed when OnTransportData is called
// the next time.
// => consider moving processing to NetworkEarlyUpdate.
while (!isLoadingScene &&
unbatcher.GetNextMessage(out ArraySegment<byte> message, out double remoteTimestamp))
//
// GetNextMessage may throw for malformed batches
// (invalid varint, message size > remaining bytes).
// catch and disconnect to prevent queue growth attacks.
try
{
using (NetworkReaderPooled reader = NetworkReaderPool.Get(message))
while (!isLoadingScene &&
unbatcher.GetNextMessage(out ArraySegment<byte> message, out double remoteTimestamp))
{
// enough to read at least header size?
if (reader.Remaining >= NetworkMessages.IdSize)
using (NetworkReaderPooled reader = NetworkReaderPool.Get(message))
{
// make remoteTimeStamp available to the user
connection.remoteTimeStamp = remoteTimestamp;
// enough to read at least header size?
if (reader.Remaining >= NetworkMessages.IdSize)
{
// make remoteTimeStamp available to the user
connection.remoteTimeStamp = remoteTimestamp;

// handle message
if (!UnpackAndInvoke(reader, channelId))
// handle message
if (!UnpackAndInvoke(reader, channelId))
{
// warn, disconnect and return if failed
// -> warning because attackers might send random data
// -> messages in a batch aren't length prefixed.
// failing to read one would cause undefined
// behaviour for every message afterwards.
// so we need to disconnect.
// -> return to avoid the below unbatches.count error.
// we already disconnected and handled it.
if (exceptionsDisconnect)
{
Debug.LogError($"NetworkClient: failed to unpack and invoke message. Disconnecting.");
connection.Disconnect();
}
else
Debug.LogWarning($"NetworkClient: failed to unpack and invoke message.");

return;
}
}
// otherwise disconnect
else
{
// warn, disconnect and return if failed
// -> warning because attackers might send random data
// -> messages in a batch aren't length prefixed.
// failing to read one would cause undefined
// behaviour for every message afterwards.
// so we need to disconnect.
// -> return to avoid the below unbatches.count error.
// we already disconnected and handled it.
if (exceptionsDisconnect)
{
Debug.LogError($"NetworkClient: failed to unpack and invoke message. Disconnecting.");
Debug.LogError($"NetworkClient: received Message was too short (messages should start with message id). Disconnecting.");
connection.Disconnect();
}
else
Debug.LogWarning($"NetworkClient: failed to unpack and invoke message.");

Debug.LogWarning("NetworkClient: received Message was too short (messages should start with message id)");
return;
}
}
// otherwise disconnect
else
{
if (exceptionsDisconnect)
{
Debug.LogError($"NetworkClient: received Message was too short (messages should start with message id). Disconnecting.");
connection.Disconnect();
}
else
Debug.LogWarning("NetworkClient: received Message was too short (messages should start with message id)");
return;
}
}
}
catch (Exception e)
{
// malformed batch: invalid varint, message size > remaining, etc.
// unbatcher already cleared batches when it detected the error.
Debug.LogError($"NetworkClient: failed to parse batch: {e.Message}. Disconnecting.");
connection.Disconnect();
return;
}

// if we weren't interrupted by a scene change,
// then all batched messages should have been processed now.
Expand All @@ -421,6 +436,10 @@ internal static void OnTransportData(ArraySegment<byte> data, int channelId)
if (!isLoadingScene && unbatcher.BatchesCount > 0)
{
Debug.LogError($"Still had {unbatcher.BatchesCount} batches remaining after processing, even though processing was not interrupted by a scene change. This should never happen, as it would cause ever growing batches.\nPossible reasons:\n* A message didn't deserialize as much as it serialized\n*There was no message handler for a message id, so the reader wasn't read until the end.");

// disconnect and clear to prevent memory leak / queue growth
unbatcher.Clear();
connection.Disconnect();
}
}
else Debug.LogError("Skipped Data message handling because connection is null.");
Expand Down
83 changes: 51 additions & 32 deletions Assets/Mirror/Core/NetworkServer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -951,54 +951,69 @@ internal static void OnTransportData(int connectionId, ArraySegment<byte> data,
// would only be processed when OnTransportData is called
// the next time.
// => consider moving processing to NetworkEarlyUpdate.
while (!isLoadingScene &&
connection.unbatcher.GetNextMessage(out ArraySegment<byte> message, out double remoteTimestamp))
//
// GetNextMessage may throw for malformed batches
// (invalid varint, message size > remaining bytes).
// catch and disconnect to prevent queue growth attacks.
try
{
using (NetworkReaderPooled reader = NetworkReaderPool.Get(message))
while (!isLoadingScene &&
connection.unbatcher.GetNextMessage(out ArraySegment<byte> message, out double remoteTimestamp))
{
// enough to read at least header size?
if (reader.Remaining >= NetworkMessages.IdSize)
using (NetworkReaderPooled reader = NetworkReaderPool.Get(message))
{
// make remoteTimeStamp available to the user
connection.remoteTimeStamp = remoteTimestamp;
// enough to read at least header size?
if (reader.Remaining >= NetworkMessages.IdSize)
{
// make remoteTimeStamp available to the user
connection.remoteTimeStamp = remoteTimestamp;

// handle message
if (!UnpackAndInvoke(connection, reader, channelId))
// handle message
if (!UnpackAndInvoke(connection, reader, channelId))
{
// warn, disconnect and return if failed
// -> warning because attackers might send random data
// -> messages in a batch aren't length prefixed.
// failing to read one would cause undefined
// behaviour for every message afterwards.
// so we need to disconnect.
// -> return to avoid the below unbatches.count error.
// we already disconnected and handled it.
if (exceptionsDisconnect)
{
Debug.LogError($"NetworkServer: failed to unpack and invoke message. Disconnecting {connectionId}.");
connection.Disconnect();
}
else
Debug.LogWarning($"NetworkServer: failed to unpack and invoke message from connectionId:{connectionId}.");

return;
}
}
// otherwise disconnect
else
{
// warn, disconnect and return if failed
// -> warning because attackers might send random data
// -> messages in a batch aren't length prefixed.
// failing to read one would cause undefined
// behaviour for every message afterwards.
// so we need to disconnect.
// -> return to avoid the below unbatches.count error.
// we already disconnected and handled it.
if (exceptionsDisconnect)
{
Debug.LogError($"NetworkServer: failed to unpack and invoke message. Disconnecting {connectionId}.");
Debug.LogError($"NetworkServer: received message from connectionId:{connectionId} was too short (messages should start with message id). Disconnecting.");
connection.Disconnect();
}
else
Debug.LogWarning($"NetworkServer: failed to unpack and invoke message from connectionId:{connectionId}.");
Debug.LogWarning($"NetworkServer: received message from connectionId:{connectionId} was too short (messages should start with message id).");

return;
}
}
// otherwise disconnect
else
{
if (exceptionsDisconnect)
{
Debug.LogError($"NetworkServer: received message from connectionId:{connectionId} was too short (messages should start with message id). Disconnecting.");
connection.Disconnect();
}
else
Debug.LogWarning($"NetworkServer: received message from connectionId:{connectionId} was too short (messages should start with message id).");

return;
}
}
}
catch (Exception e)
{
// malformed batch: invalid varint, message size > remaining, etc.
// unbatcher already cleared batches when it detected the error.
Debug.LogError($"NetworkServer: failed to parse batch from connectionId:{connectionId}: {e.Message}. Disconnecting.");
connection.Disconnect();
return;
}

// if we weren't interrupted by a scene change,
// then all batched messages should have been processed now.
Expand All @@ -1018,6 +1033,10 @@ internal static void OnTransportData(int connectionId, ArraySegment<byte> data,
if (!isLoadingScene && connection.unbatcher.BatchesCount > 0)
{
Debug.LogError($"Still had {connection.unbatcher.BatchesCount} batches remaining after processing, even though processing was not interrupted by a scene change. This should never happen, as it would cause ever growing batches.\nPossible reasons:\n* A message didn't deserialize as much as it serialized\n*There was no message handler for a message id, so the reader wasn't read until the end.");

// disconnect and clear to prevent memory leak / queue growth
connection.unbatcher.Clear();
connection.Disconnect();
}
}
else Debug.LogError($"HandleData Unknown connectionId:{connectionId}");
Expand Down
77 changes: 77 additions & 0 deletions Assets/Mirror/Tests/Editor/Batching/UnbatcherTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,5 +133,82 @@ public void RetireBatchAndTryNewBatch()
Assert.That(reader.ReadByte(), Is.EqualTo(0x04));
Assert.That(remoteTimeStamp, Is.EqualTo(TimeStamp + 1));
}

// malformed batch: message size prefix larger than remaining bytes.
// GetNextMessage should throw and clear all batches.
[Test]
public void GetNextMessage_MalformedBatch_SizeLargerThanRemaining()
{
// craft a batch with timestamp + varint size prefix claiming 1,000,000 bytes
// but only a few bytes actually remaining
NetworkWriter writer = new NetworkWriter();
writer.WriteDouble(TimeStamp);
Compression.CompressVarUInt(writer, 1000000); // claim 1M bytes
writer.WriteByte(0xFF); // but only 1 byte of data
byte[] batch = writer.ToArray();

unbatcher.AddBatch(new ArraySegment<byte>(batch));
Assert.That(unbatcher.BatchesCount, Is.EqualTo(1));

// GetNextMessage should throw InvalidOperationException
Assert.Throws<InvalidOperationException>(() =>
{
unbatcher.GetNextMessage(out _, out _);
});

// all batches should be cleared
Assert.That(unbatcher.BatchesCount, Is.EqualTo(0));
}

// malformed batch should not allow queue growth when repeated
[Test]
public void GetNextMessage_MalformedBatch_NoQueueGrowth()
{
// add multiple malformed batches
for (int i = 0; i < 10; i++)
{
NetworkWriter writer = new NetworkWriter();
writer.WriteDouble(TimeStamp);
Compression.CompressVarUInt(writer, 1000000);
writer.WriteByte(0xFF);
byte[] batch = writer.ToArray();

unbatcher.AddBatch(new ArraySegment<byte>(batch));

// each GetNextMessage should throw and clear
Assert.Throws<InvalidOperationException>(() =>
{
unbatcher.GetNextMessage(out _, out _);
});

// batches should be cleared after each malformed batch
Assert.That(unbatcher.BatchesCount, Is.EqualTo(0));
}
}

[Test]
public void Clear()
{
// add batches
byte[] batch1 = BatcherTests.MakeBatch(TimeStamp, new byte[] {0x01, 0x02});
byte[] batch2 = BatcherTests.MakeBatch(TimeStamp + 1, new byte[] {0x03, 0x04});
unbatcher.AddBatch(new ArraySegment<byte>(batch1));
unbatcher.AddBatch(new ArraySegment<byte>(batch2));
Assert.That(unbatcher.BatchesCount, Is.EqualTo(2));

// clear should remove all batches
unbatcher.Clear();
Assert.That(unbatcher.BatchesCount, Is.EqualTo(0));

// should be able to add and read new batches after clear
byte[] batch3 = BatcherTests.MakeBatch(TimeStamp + 2, new byte[] {0x05, 0x06});
unbatcher.AddBatch(new ArraySegment<byte>(batch3));
bool result = unbatcher.GetNextMessage(out ArraySegment<byte> message, out double remoteTimeStamp);
Assert.That(result, Is.True);
NetworkReader reader = new NetworkReader(message);
Assert.That(reader.ReadByte(), Is.EqualTo(0x05));
Assert.That(reader.ReadByte(), Is.EqualTo(0x06));
Assert.That(remoteTimeStamp, Is.EqualTo(TimeStamp + 2));
}
}
}
Loading