diff --git a/src/libraries/System.Formats.Nrbf/src/Resources/Strings.resx b/src/libraries/System.Formats.Nrbf/src/Resources/Strings.resx index 349405150c65e..6b9ffc6da372b 100644 --- a/src/libraries/System.Formats.Nrbf/src/Resources/Strings.resx +++ b/src/libraries/System.Formats.Nrbf/src/Resources/Strings.resx @@ -133,7 +133,7 @@ {0} Record Type is not supported by design. - Member reference was pointing to a record of unexpected type. + Invalid member reference. Invalid type name: `{0}`. @@ -162,4 +162,10 @@ Invalid assembly name: `{0}`. + + Invalid format. + + + A surrogate character was read. + \ No newline at end of file diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySinglePrimitiveRecord.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySinglePrimitiveRecord.cs index 6b97328702bd3..ab5f68f50be82 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySinglePrimitiveRecord.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ArraySinglePrimitiveRecord.cs @@ -171,12 +171,17 @@ private static List DecodeDecimals(BinaryReader reader, int count) reader.BaseStream.ReadExactly(buffer.Slice(0, stringLength)); - values.Add(decimal.Parse(buffer.Slice(0, stringLength), CultureInfo.InvariantCulture)); + if (!decimal.TryParse(buffer.Slice(0, stringLength), NumberStyles.Number, CultureInfo.InvariantCulture, out decimal value)) + { + ThrowHelper.ThrowInvalidFormat(); + } + + values.Add(value); } #else for (int i = 0; i < count; i++) { - values.Add(decimal.Parse(reader.ReadString(), CultureInfo.InvariantCulture)); + values.Add(reader.ParseDecimal()); } #endif return values; diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ClassTypeInfo.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ClassTypeInfo.cs index 6a9e9d7b90afe..b0b7e543fa9b5 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ClassTypeInfo.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ClassTypeInfo.cs @@ -26,7 +26,7 @@ internal static ClassTypeInfo Decode(BinaryReader reader, PayloadOptions options string rawName = reader.ReadString(); SerializationRecordId libraryId = SerializationRecordId.Decode(reader); - BinaryLibraryRecord library = (BinaryLibraryRecord)recordMap[libraryId]; + BinaryLibraryRecord library = recordMap.GetRecord(libraryId); return new ClassTypeInfo(rawName.ParseNonSystemClassRecordTypeName(library, options)); } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ClassWithIdRecord.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ClassWithIdRecord.cs index e18033524d17e..c643d3ce8c846 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ClassWithIdRecord.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ClassWithIdRecord.cs @@ -34,10 +34,7 @@ internal static ClassWithIdRecord Decode( SerializationRecordId id = SerializationRecordId.Decode(reader); SerializationRecordId metadataId = SerializationRecordId.Decode(reader); - if (recordMap[metadataId] is not ClassRecord referencedRecord) - { - throw new SerializationException(SR.Serialization_InvalidReference); - } + ClassRecord referencedRecord = recordMap.GetRecord(metadataId); return new ClassWithIdRecord(id, referencedRecord); } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ClassWithMembersAndTypesRecord.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ClassWithMembersAndTypesRecord.cs index 117e5e90ef681..d6d8c122d3ed9 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ClassWithMembersAndTypesRecord.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/ClassWithMembersAndTypesRecord.cs @@ -27,7 +27,7 @@ internal static ClassWithMembersAndTypesRecord Decode(BinaryReader reader, Recor MemberTypeInfo memberTypeInfo = MemberTypeInfo.Decode(reader, classInfo.MemberNames.Count, options, recordMap); SerializationRecordId libraryId = SerializationRecordId.Decode(reader); - BinaryLibraryRecord library = (BinaryLibraryRecord)recordMap[libraryId]; + BinaryLibraryRecord library = recordMap.GetRecord(libraryId); classInfo.LoadTypeName(library, options); return new ClassWithMembersAndTypesRecord(classInfo, memberTypeInfo); diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberReferenceRecord.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberReferenceRecord.cs index 162cf0b1d5c57..14bd4e7ff1f2d 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberReferenceRecord.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/MemberReferenceRecord.cs @@ -38,5 +38,5 @@ private MemberReferenceRecord(SerializationRecordId reference, RecordMap recordM internal static MemberReferenceRecord Decode(BinaryReader reader, RecordMap recordMap) => new(SerializationRecordId.Decode(reader), recordMap); - internal SerializationRecord GetReferencedRecord() => RecordMap[Reference]; + internal SerializationRecord GetReferencedRecord() => RecordMap.GetRecord(Reference); } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NrbfDecoder.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NrbfDecoder.cs index 6eb69eb658b9f..de4b24b6e46e1 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NrbfDecoder.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/NrbfDecoder.cs @@ -46,6 +46,7 @@ public static bool StartsWithPayloadHeader(ReadOnlySpan bytes) /// is . /// The stream does not support reading or seeking. /// The stream was closed. + /// An I/O error occurred. /// When this method returns, is restored to its original position. public static bool StartsWithPayloadHeader(Stream stream) { @@ -107,6 +108,7 @@ public static bool StartsWithPayloadHeader(Stream stream) /// is . /// does not support reading or is already closed. /// Reading from encountered invalid NRBF data. + /// An I/O error occurred. /// /// Reading from encountered unsupported records, /// for example, arrays with non-zero offset or unsupported record types @@ -142,7 +144,14 @@ public static SerializationRecord Decode(Stream payload, out IReadOnlyDictionary #endif using BinaryReader reader = new(payload, ThrowOnInvalidUtf8Encoding, leaveOpen: leaveOpen); - return Decode(reader, options ?? new(), out recordMap); + try + { + return Decode(reader, options ?? new(), out recordMap); + } + catch (FormatException) // can be thrown by various BinaryReader methods + { + throw new SerializationException(SR.Serialization_InvalidFormat); + } } /// @@ -213,12 +222,7 @@ private static SerializationRecord Decode(BinaryReader reader, PayloadOptions op private static SerializationRecord DecodeNext(BinaryReader reader, RecordMap recordMap, AllowedRecordTypes allowed, PayloadOptions options, out SerializationRecordType recordType) { - byte nextByte = reader.ReadByte(); - if (((uint)allowed & (1u << nextByte)) == 0) - { - ThrowHelper.ThrowForUnexpectedRecordType(nextByte); - } - recordType = (SerializationRecordType)nextByte; + recordType = reader.ReadSerializationRecordType(allowed); SerializationRecord record = recordType switch { @@ -254,7 +258,7 @@ private static SerializationRecord DecodeMemberPrimitiveTypedRecord(BinaryReader PrimitiveType.Boolean => new MemberPrimitiveTypedRecord(reader.ReadBoolean()), PrimitiveType.Byte => new MemberPrimitiveTypedRecord(reader.ReadByte()), PrimitiveType.SByte => new MemberPrimitiveTypedRecord(reader.ReadSByte()), - PrimitiveType.Char => new MemberPrimitiveTypedRecord(reader.ReadChar()), + PrimitiveType.Char => new MemberPrimitiveTypedRecord(reader.ParseChar()), PrimitiveType.Int16 => new MemberPrimitiveTypedRecord(reader.ReadInt16()), PrimitiveType.UInt16 => new MemberPrimitiveTypedRecord(reader.ReadUInt16()), PrimitiveType.Int32 => new MemberPrimitiveTypedRecord(reader.ReadInt32()), @@ -263,7 +267,7 @@ private static SerializationRecord DecodeMemberPrimitiveTypedRecord(BinaryReader PrimitiveType.UInt64 => new MemberPrimitiveTypedRecord(reader.ReadUInt64()), PrimitiveType.Single => new MemberPrimitiveTypedRecord(reader.ReadSingle()), PrimitiveType.Double => new MemberPrimitiveTypedRecord(reader.ReadDouble()), - PrimitiveType.Decimal => new MemberPrimitiveTypedRecord(decimal.Parse(reader.ReadString(), CultureInfo.InvariantCulture)), + PrimitiveType.Decimal => new MemberPrimitiveTypedRecord(reader.ParseDecimal()), PrimitiveType.DateTime => new MemberPrimitiveTypedRecord(Utils.BinaryReaderExtensions.CreateDateTimeFromData(reader.ReadUInt64())), // String is handled with a record, never on it's own _ => new MemberPrimitiveTypedRecord(new TimeSpan(reader.ReadInt64())), diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/RecordMap.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/RecordMap.cs index a25ab508f5db3..04a4d0e085048 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/RecordMap.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/RecordMap.cs @@ -63,7 +63,8 @@ internal void Add(SerializationRecord record) internal SerializationRecord GetRootRecord(SerializedStreamHeaderRecord header) { - SerializationRecord rootRecord = _map[header.RootId]; + SerializationRecord rootRecord = GetRecord(header.RootId); + if (rootRecord is SystemClassWithMembersAndTypesRecord systemClass) { // update the record map, so it's visible also to those who access it via Id @@ -72,4 +73,14 @@ internal SerializationRecord GetRootRecord(SerializedStreamHeaderRecord header) return rootRecord; } + + internal SerializationRecord GetRecord(SerializationRecordId recordId) + => _map.TryGetValue(recordId, out SerializationRecord? record) + ? record + : throw new SerializationException(SR.Serialization_InvalidReference); + + internal T GetRecord(SerializationRecordId recordId) where T : SerializationRecord + => _map.TryGetValue(recordId, out SerializationRecord? record) && record is T casted + ? casted + : throw new SerializationException(SR.Serialization_InvalidReference); } diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/BinaryReaderExtensions.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/BinaryReaderExtensions.cs index 5cd81be6d9995..cd705223021fc 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/BinaryReaderExtensions.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/BinaryReaderExtensions.cs @@ -15,6 +15,19 @@ internal static class BinaryReaderExtensions { private static object? s_baseAmbiguousDstDateTime; + internal static SerializationRecordType ReadSerializationRecordType(this BinaryReader reader, AllowedRecordTypes allowed) + { + byte nextByte = reader.ReadByte(); + if (nextByte > (byte)SerializationRecordType.MethodReturn // MethodReturn is the last defined value. + || (nextByte > (byte)SerializationRecordType.ArraySingleString && nextByte < (byte)SerializationRecordType.MethodCall) // not part of the spec + || ((uint)allowed & (1u << nextByte)) == 0) // valid, but not allowed + { + ThrowHelper.ThrowForUnexpectedRecordType(nextByte); + } + + return (SerializationRecordType)nextByte; + } + internal static BinaryArrayType ReadArrayType(this BinaryReader reader) { byte arrayType = reader.ReadByte(); @@ -48,7 +61,7 @@ internal static PrimitiveType ReadPrimitiveType(this BinaryReader reader) { byte primitiveType = reader.ReadByte(); // String is the last defined value, 4 is not used at all. - if (primitiveType is 4 or > (byte)PrimitiveType.String) + if (primitiveType is 0 or 4 or (byte)PrimitiveType.Null or > (byte)PrimitiveType.String) { ThrowHelper.ThrowInvalidValue(primitiveType); } @@ -64,7 +77,7 @@ internal static object ReadPrimitiveValue(this BinaryReader reader, PrimitiveTyp PrimitiveType.Boolean => reader.ReadBoolean(), PrimitiveType.Byte => reader.ReadByte(), PrimitiveType.SByte => reader.ReadSByte(), - PrimitiveType.Char => reader.ReadChar(), + PrimitiveType.Char => reader.ParseChar(), PrimitiveType.Int16 => reader.ReadInt16(), PrimitiveType.UInt16 => reader.ReadUInt16(), PrimitiveType.Int32 => reader.ReadInt32(), @@ -73,11 +86,35 @@ internal static object ReadPrimitiveValue(this BinaryReader reader, PrimitiveTyp PrimitiveType.UInt64 => reader.ReadUInt64(), PrimitiveType.Single => reader.ReadSingle(), PrimitiveType.Double => reader.ReadDouble(), - PrimitiveType.Decimal => decimal.Parse(reader.ReadString(), CultureInfo.InvariantCulture), + PrimitiveType.Decimal => reader.ParseDecimal(), PrimitiveType.DateTime => CreateDateTimeFromData(reader.ReadUInt64()), _ => new TimeSpan(reader.ReadInt64()), }; + // BinaryFormatter serializes decimals as strings and we can't BinaryReader.ReadDecimal. + internal static decimal ParseDecimal(this BinaryReader reader) + { + string text = reader.ReadString(); + if (!decimal.TryParse(text, NumberStyles.Number, CultureInfo.InvariantCulture, out decimal result)) + { + ThrowHelper.ThrowInvalidFormat(); + } + + return result; + } + + internal static char ParseChar(this BinaryReader reader) + { + try + { + return reader.ReadChar(); + } + catch (ArgumentException) // A surrogate character was read. + { + throw new SerializationException(SR.Serialization_SurrogateCharacter); + } + } + /// /// Creates a object from raw data with validation. /// diff --git a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/ThrowHelper.cs b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/ThrowHelper.cs index f096bfc736098..55febf77533f9 100644 --- a/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/ThrowHelper.cs +++ b/src/libraries/System.Formats.Nrbf/src/System/Formats/Nrbf/Utils/ThrowHelper.cs @@ -29,6 +29,9 @@ internal static void ThrowArrayContainedNulls() internal static void ThrowInvalidAssemblyName(string rawName) => throw new SerializationException(SR.Format(SR.Serialization_InvalidAssemblyName, rawName)); + internal static void ThrowInvalidFormat() + => throw new SerializationException(SR.Serialization_InvalidFormat); + internal static void ThrowEndOfStreamException() => throw new EndOfStreamException(); diff --git a/src/libraries/System.Formats.Nrbf/tests/InvalidInputTests.cs b/src/libraries/System.Formats.Nrbf/tests/InvalidInputTests.cs index bc134350eb7c9..b1625c7ca9287 100644 --- a/src/libraries/System.Formats.Nrbf/tests/InvalidInputTests.cs +++ b/src/libraries/System.Formats.Nrbf/tests/InvalidInputTests.cs @@ -426,7 +426,9 @@ public static IEnumerable ThrowsForInvalidPrimitiveType_Arguments() { foreach (byte binaryType in new byte[] { (byte)0 /* BinaryType.Primitive */, (byte)7 /* BinaryType.PrimitiveArray */ }) { + yield return new object[] { recordType, binaryType, (byte)0 }; // value not used by the spec yield return new object[] { recordType, binaryType, (byte)4 }; // value not used by the spec + yield return new object[] { recordType, binaryType, (byte)17 }; // used by the spec, but illegal in given context yield return new object[] { recordType, binaryType, (byte)19 }; } } @@ -478,4 +480,109 @@ public void ThrowsOnInvalidArrayType() stream.Position = 0; Assert.Throws(() => NrbfDecoder.Decode(stream)); } + + [Theory] + [InlineData(18, typeof(NotSupportedException))] // not part of the spec, but still less than max allowed value (22) + [InlineData(19, typeof(NotSupportedException))] // same as above + [InlineData(20, typeof(NotSupportedException))] // same as above + [InlineData(23, typeof(SerializationException))] // not part of the spec and more than max allowed value (22) + [InlineData(64, typeof(SerializationException))] // same as above but also matches AllowedRecordTypes.SerializedStreamHeader + public void InvalidSerializationRecordType(byte recordType, Type expectedException) + { + using MemoryStream stream = new(); + BinaryWriter writer = new(stream, Encoding.UTF8); + + WriteSerializedStreamHeader(writer); + writer.Write(recordType); // SerializationRecordType + writer.Write((byte)SerializationRecordType.MessageEnd); + + stream.Position = 0; + + Assert.Throws(expectedException, () => NrbfDecoder.Decode(stream)); + } + + [Fact] + public void MissingRootRecord() + { + const int RootRecordId = 1; + using MemoryStream stream = new(); + BinaryWriter writer = new(stream, Encoding.UTF8); + + WriteSerializedStreamHeader(writer, rootId: RootRecordId); + writer.Write((byte)SerializationRecordType.BinaryObjectString); + writer.Write(RootRecordId + 1); // a different ID + writer.Write("theString"); + writer.Write((byte)SerializationRecordType.MessageEnd); + + stream.Position = 0; + + Assert.Throws(() => NrbfDecoder.Decode(stream)); + } + + [Fact] + public void Invalid7BitEncodedStringLength() + { + // The highest bit of the last byte is set (so it's invalid). + byte[] invalidLength = [byte.MaxValue, byte.MaxValue, byte.MaxValue, byte.MaxValue, byte.MaxValue]; + + using MemoryStream stream = new(); + BinaryWriter writer = new(stream, Encoding.UTF8); + + WriteSerializedStreamHeader(writer); + writer.Write((byte)SerializationRecordType.BinaryObjectString); + writer.Write(1); // root record Id + writer.Write(invalidLength); // the length prefix + writer.Write(Encoding.UTF8.GetBytes("theString")); + writer.Write((byte)SerializationRecordType.MessageEnd); + + stream.Position = 0; + + Assert.Throws(() => NrbfDecoder.Decode(stream)); + } + + [Theory] + [InlineData("79228162514264337593543950336")] // invalid format (decimal.MaxValue + 1) + [InlineData("1111111111111111111111111111111111111111111111111")] // overflow + public void InvalidDecimal(string textRepresentation) + { + using MemoryStream stream = new(); + BinaryWriter writer = new(stream, Encoding.UTF8); + + WriteSerializedStreamHeader(writer); + writer.Write((byte)SerializationRecordType.SystemClassWithMembersAndTypes); + writer.Write(1); // root record Id + writer.Write("ClassWithDecimalField"); // type name + writer.Write(1); // member count + writer.Write("memberName"); + writer.Write((byte)BinaryType.Primitive); + writer.Write((byte)PrimitiveType.Decimal); + writer.Write(textRepresentation); + writer.Write((byte)SerializationRecordType.MessageEnd); + + stream.Position = 0; + + Assert.Throws(() => NrbfDecoder.Decode(stream)); + } + + [Fact] + public void SurrogateCharacter() + { + using MemoryStream stream = new(); + BinaryWriter writer = new(stream, Encoding.UTF8); + + WriteSerializedStreamHeader(writer); + writer.Write((byte)SerializationRecordType.SystemClassWithMembersAndTypes); + writer.Write(1); // root record Id + writer.Write("ClassWithCharField"); // type name + writer.Write(1); // member count + writer.Write("memberName"); + writer.Write((byte)BinaryType.Primitive); + writer.Write((byte)PrimitiveType.Char); + writer.Write((byte)0xC0); // a surrogate character + writer.Write((byte)SerializationRecordType.MessageEnd); + + stream.Position = 0; + + Assert.Throws(() => NrbfDecoder.Decode(stream)); + } } diff --git a/src/libraries/System.Formats.Nrbf/tests/ReadTests.cs b/src/libraries/System.Formats.Nrbf/tests/ReadTests.cs index b9bee7d881a69..0c7bd2045fa1f 100644 --- a/src/libraries/System.Formats.Nrbf/tests/ReadTests.cs +++ b/src/libraries/System.Formats.Nrbf/tests/ReadTests.cs @@ -45,10 +45,10 @@ protected static BinaryFormatter CreateBinaryFormatter() }; #pragma warning restore SYSLIB0011 // Type or member is obsolete - protected static void WriteSerializedStreamHeader(BinaryWriter writer, int major = 1, int minor = 0) + protected static void WriteSerializedStreamHeader(BinaryWriter writer, int major = 1, int minor = 0, int rootId = 1) { writer.Write((byte)SerializationRecordType.SerializedStreamHeader); - writer.Write(1); // root ID + writer.Write(rootId); // root ID writer.Write(1); // header ID writer.Write(major); // major version writer.Write(minor); // minor version diff --git a/src/libraries/System.Formats.Nrbf/tests/System.Formats.Nrbf.Tests.csproj b/src/libraries/System.Formats.Nrbf/tests/System.Formats.Nrbf.Tests.csproj index a9da043b5ecd5..c31537bb983ef 100644 --- a/src/libraries/System.Formats.Nrbf/tests/System.Formats.Nrbf.Tests.csproj +++ b/src/libraries/System.Formats.Nrbf/tests/System.Formats.Nrbf.Tests.csproj @@ -7,6 +7,8 @@ + +