From 5832f688e03441a212c241011c9c4e3c6382074c Mon Sep 17 00:00:00 2001
From: Adam Sitnik <adam.sitnik@gmail.com>
Date: Tue, 10 Sep 2024 10:28:08 +0200
Subject: [PATCH] [NRBF] throw SerializationException when a surrogate
 character is read (#107532)

 (so far an ArgumentException was thrown)
---
 .../Nrbf/ArraySinglePrimitiveRecord.cs        |  4 ++--
 .../Nrbf/Utils/BinaryReaderExtensions.cs      | 12 ++++++++++
 .../tests/InvalidInputTests.cs                | 24 +++++++++++++++----
 3 files changed, 34 insertions(+), 6 deletions(-)

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 ab5f68f50be820..5b3b9746394680 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
@@ -85,7 +85,7 @@ internal static IReadOnlyList<T> DecodePrimitiveTypes(BinaryReader reader, int c
         }
         else if (typeof(T) == typeof(char))
         {
-            return (T[])(object)reader.ReadChars(count);
+            return (T[])(object)reader.ParseChars(count);
         }
 
         // It's safe to pre-allocate, as we have ensured there is enough bytes in the stream.
@@ -206,7 +206,7 @@ private static List<T> DecodeFromNonSeekableStream(BinaryReader reader, int coun
             }
             else if (typeof(T) == typeof(char))
             {
-                values.Add((T)(object)reader.ReadChar());
+                values.Add((T)(object)reader.ParseChar());
             }
             else if (typeof(T) == typeof(short))
             {
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 cd705223021fcc..ff422c29401a12 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
@@ -115,6 +115,18 @@ internal static char ParseChar(this BinaryReader reader)
         }
     }
 
+    internal static char[] ParseChars(this BinaryReader reader, int count)
+    {
+        try
+        {
+            return reader.ReadChars(count);
+        }
+        catch (ArgumentException) // A surrogate character was read.
+        {
+            throw new SerializationException(SR.Serialization_SurrogateCharacter);
+        }
+    }
+
     /// <summary>
     ///  Creates a <see cref="DateTime"/> object from raw data with validation.
     /// </summary>
diff --git a/src/libraries/System.Formats.Nrbf/tests/InvalidInputTests.cs b/src/libraries/System.Formats.Nrbf/tests/InvalidInputTests.cs
index b1625c7ca92870..81899523f7d9dc 100644
--- a/src/libraries/System.Formats.Nrbf/tests/InvalidInputTests.cs
+++ b/src/libraries/System.Formats.Nrbf/tests/InvalidInputTests.cs
@@ -564,8 +564,10 @@ public void InvalidDecimal(string textRepresentation)
         Assert.Throws<SerializationException>(() => NrbfDecoder.Decode(stream));
     }
 
-    [Fact]
-    public void SurrogateCharacter()
+    [Theory]
+    [InlineData(true)]
+    [InlineData(false)]
+    public void SurrogateCharacters(bool array)
     {
         using MemoryStream stream = new();
         BinaryWriter writer = new(stream, Encoding.UTF8);
@@ -576,8 +578,22 @@ public void SurrogateCharacter()
         writer.Write("ClassWithCharField"); // type name
         writer.Write(1); // member count
         writer.Write("memberName");
-        writer.Write((byte)BinaryType.Primitive);
-        writer.Write((byte)PrimitiveType.Char);
+
+        if (array)
+        {
+            writer.Write((byte)BinaryType.PrimitiveArray);
+            writer.Write((byte)PrimitiveType.Char);
+            writer.Write((byte)SerializationRecordType.ArraySinglePrimitive);
+            writer.Write(2); // array record Id
+            writer.Write(1); // array length
+            writer.Write((byte)PrimitiveType.Char);
+        }
+        else
+        {
+            writer.Write((byte)BinaryType.Primitive);
+            writer.Write((byte)PrimitiveType.Char);
+        }
+
         writer.Write((byte)0xC0); // a surrogate character
         writer.Write((byte)SerializationRecordType.MessageEnd);