Skip to content

Commit 6133d41

Browse files
test
This test validates the fix.
1 parent 259891a commit 6133d41

1 file changed

Lines changed: 34 additions & 0 deletions

File tree

com.unity.netcode.gameobjects/Tests/Editor/Serialization/FastBufferReaderTests.cs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -924,6 +924,40 @@ public void GivenFastBufferWriterContainingValue_WhenReadingString_ValueMatchesW
924924
}
925925
}
926926

927+
/// <summary>
928+
/// This validates that <see cref="FastBufferReader"/> catches a potential
929+
/// scenario where the string's character count value has already been read
930+
/// due to an error within user script and the resultant character count
931+
/// multiplied times 2 (when using 2 bytes vs 1) causes the length to roll over
932+
/// to a negative value which, in turn, causes the reader to attempt to read
933+
/// into restricted memory and causes the editor to crash.
934+
/// </summary>
935+
[Test]
936+
public void ReadingStringAfterStringLengthHasAlreadyBeenRead()
937+
{
938+
// This was an issue uncovered in UUM-145752 that resulted
939+
// in the below text to result in a length that when using
940+
// 2 bytes per character would cause the skewed size to roll
941+
// over into a negative value causing the editor to crash
942+
// when it attempted to read a large negative offset value.
943+
string valueToTest = "true";
944+
945+
var serializedValueSize = FastBufferWriter.GetWriteSize(valueToTest);
946+
947+
using var writer = new FastBufferWriter(serializedValueSize + 3, Allocator.Temp);
948+
writer.WriteValueSafe(valueToTest);
949+
950+
using var reader = new FastBufferReader(writer, Allocator.Temp);
951+
952+
// Read the value of the character count before trying to read the string
953+
// This mocks user code having read too far into a stream causing the position to be skewed such that
954+
// the string reader reads the some of the bytes for the actual text as the length.
955+
reader.ReadByteSafe(out byte count);
956+
Assert.True(count == valueToTest.Length, $"Count ({count}) is not the expected size of {valueToTest.Length}!");
957+
958+
// This should throw an overflow exception but should not crash the editor.
959+
Assert.Throws<OverflowException>(() => reader.ReadValueSafe(out string valueRead));
960+
}
927961

928962
[TestCase(1, 0)]
929963
[TestCase(2, 0)]

0 commit comments

Comments
 (0)