From c8aa761a71459dccc20be20a3076fb36cb4fcc6e Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Thu, 16 Jul 2026 11:44:35 -0500 Subject: [PATCH 1/4] fix Adding bounds check for reading NetworkAnimator parameters. --- .../Runtime/Components/NetworkAnimator.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/com.unity.netcode.gameobjects/Runtime/Components/NetworkAnimator.cs b/com.unity.netcode.gameobjects/Runtime/Components/NetworkAnimator.cs index cdb25aa598..c999c26e9f 100644 --- a/com.unity.netcode.gameobjects/Runtime/Components/NetworkAnimator.cs +++ b/com.unity.netcode.gameobjects/Runtime/Components/NetworkAnimator.cs @@ -1377,6 +1377,13 @@ private unsafe void ReadParameters(FastBufferReader reader) while (totalParametersRead < totalParametersToRead) { ByteUnpacker.ReadValuePacked(reader, out uint parameterIndex); + + // Do bounds check prior to getting the element as a reference at that index. + if (parameterIndex >= m_CachedAnimatorParameters.Length) + { + NetworkManager.Log.ErrorServer(new Logging.Context(LogLevel.Error, $"[{nameof(NetworkAnimator)}][{name}] Invalid index of {parameterIndex} was received when there are only {m_CachedAnimatorParameters.Length} parameters. Ignoring the remainger of this {nameof(ParametersUpdateMessage)}!")); + return; + } ref var cacheValue = ref UnsafeUtility.ArrayElementAsRef(m_CachedAnimatorParameters.GetUnsafePtr(), (int)parameterIndex); var hash = cacheValue.Hash; if (cacheValue.Type == AnimationParamEnumWrapper.AnimatorControllerParameterInt) From 500a163f09b16d316fecf29e746ce5f2c2ed6a6e Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Thu, 16 Jul 2026 11:44:54 -0500 Subject: [PATCH 2/4] test Adding a unit test to validate the bounds check. --- .../Runtime/Animation/NetworkAnimatorTests.cs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/testproject/Assets/Tests/Runtime/Animation/NetworkAnimatorTests.cs b/testproject/Assets/Tests/Runtime/Animation/NetworkAnimatorTests.cs index 715e7f6156..e258a2aa7e 100644 --- a/testproject/Assets/Tests/Runtime/Animation/NetworkAnimatorTests.cs +++ b/testproject/Assets/Tests/Runtime/Animation/NetworkAnimatorTests.cs @@ -3,9 +3,11 @@ using System.Linq; using NUnit.Framework; using Unity.Netcode; +using Unity.Netcode.Components; using Unity.Netcode.TestHelpers.Runtime; using UnityEngine; using UnityEngine.TestTools; +using static Unity.Netcode.Components.NetworkAnimator; namespace TestProject.RuntimeTests @@ -336,7 +338,35 @@ public void ParameterExcludedTests() VerboseDebug($" ------------------ Parameter Test [{m_OwnerShipMode}] Stopping ------------------ "); } + private unsafe void MockWritingParameters(ref FastBufferWriter writer) + { + writer.Seek(0); + writer.Truncate(); + // Write out how many parameter entries to read + BytePacker.WriteValuePacked(writer, (uint)1); + BytePacker.WriteValuePacked(writer, (uint)1000); + BytePacker.WriteValuePacked(writer, (uint)10); + } + + [Test] + public void ParameterBoundsCheck() + { + var gameObject = new GameObject(); + gameObject.AddComponent(); + var networkAnimator = gameObject.AddComponent(); + var writer = new FastBufferWriter(40, Unity.Collections.Allocator.TempJob); + + MockWritingParameters(ref writer); + + var invalidParameters = new ParametersUpdateMessage() + { + Parameters = writer.ToArray() + }; + + LogAssert.Expect(LogType.Error, new System.Text.RegularExpressions.Regex($"parameters. Ignoring the remainger of this {nameof(ParametersUpdateMessage)}!")); + networkAnimator.UpdateParameters(ref invalidParameters); + } private bool AllTriggersDetected(OwnerShipMode ownerShipMode) { From da8b7a0266addbc5072e0df4427d3052a3fbe017 Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Thu, 16 Jul 2026 11:50:46 -0500 Subject: [PATCH 3/4] update adding change log entry --- com.unity.netcode.gameobjects/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index ff9f37f6a8..045b685fc8 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -22,6 +22,7 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Fixed +- Issue where `NetworkAnimator` did no bounds check on the parameter index read prior to obtaining a pointer to the location within the array. (#4090) - Issue where scene migration in a distributed authority session would throw an exception on clients migrating their owned `NetworkObject` instances to a different scene.(#4086) - Issue where migrating a dynamically spawned or in-scene placed `NetworkObject` into a different scene during awake could result in that spawned instance to not be synchronized. (#4086) - Issue where a NullReferenceException was thrown when a non-authority failed to spawn a NetworkObject. (#4067) From 9452e562119eb1a07e9aabbd97001f1a52078398 Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Thu, 16 Jul 2026 11:55:49 -0500 Subject: [PATCH 4/4] style adding additional comments for clarity on what the test is doing. --- .../Assets/Tests/Runtime/Animation/NetworkAnimatorTests.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/testproject/Assets/Tests/Runtime/Animation/NetworkAnimatorTests.cs b/testproject/Assets/Tests/Runtime/Animation/NetworkAnimatorTests.cs index e258a2aa7e..d685fb68ca 100644 --- a/testproject/Assets/Tests/Runtime/Animation/NetworkAnimatorTests.cs +++ b/testproject/Assets/Tests/Runtime/Animation/NetworkAnimatorTests.cs @@ -342,9 +342,12 @@ private unsafe void MockWritingParameters(ref FastBufferWriter writer) { writer.Seek(0); writer.Truncate(); + // Write out how many parameter entries to read BytePacker.WriteValuePacked(writer, (uint)1); + // Write an invalid index level (anything would be invalid with no parameters) BytePacker.WriteValuePacked(writer, (uint)1000); + // Write some value for the invalid parameter BytePacker.WriteValuePacked(writer, (uint)10); } @@ -357,14 +360,16 @@ public void ParameterBoundsCheck() var writer = new FastBufferWriter(40, Unity.Collections.Allocator.TempJob); + // Mock a parameter update with an invalid index MockWritingParameters(ref writer); var invalidParameters = new ParametersUpdateMessage() { Parameters = writer.ToArray() }; - + // Expect an error message LogAssert.Expect(LogType.Error, new System.Text.RegularExpressions.Regex($"parameters. Ignoring the remainger of this {nameof(ParametersUpdateMessage)}!")); + // Pass in the invalid ParametersUpdateMessage networkAnimator.UpdateParameters(ref invalidParameters); }