Ensure stack allocations are gaurded

drewnoakes · Feb 19, 2024 · dd06d8a · dd06d8a
1 parent 1cb62e7
commit dd06d8a
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 15 deletions.
diff --git a/MetadataExtractor/Formats/Apple/BplistReader.cs b/MetadataExtractor/Formats/Apple/BplistReader.cs
@@ -54,7 +54,7 @@ public static PropertyListResults Parse(ReadOnlySpan<byte> bplist)
         {
             for (int i = 0; i < offsets.Length; i++)
             {
-                offsets[i] = BinaryPrimitives.ReadUInt16BigEndian(offsetsBytes.Slice(i * 2, 2));
+                offsets[i] = BinaryPrimitives.ReadInt16BigEndian(offsetsBytes.Slice(i * 2, 2));
             }
         }
 
@@ -133,7 +133,7 @@ static Dictionary<byte, byte> HandleDict(ref BufferReader reader, byte count)
             return map;
         }
 
-        static object HandleData(ref BufferReader reader, byte marker)
+        object HandleData(ref BufferReader reader, byte marker)
         {
             int byteCount = marker;
 

diff --git a/MetadataExtractor/Formats/Bmp/BmpReader.cs b/MetadataExtractor/Formats/Bmp/BmpReader.cs
@@ -347,15 +347,22 @@ private static void ReadBitmapHeader(SequentialReader reader, BmpHeaderDirectory
                         }
                         else
                         {
-                            using var iccBuffer = profileSize <= 256
-                                ? new BufferScope(stackalloc byte[profileSize])
-                                : new BufferScope(profileSize);
+                            if (profileSize <= 0)
+                            {
+                                directory.AddError($"Invalid ICC profile size. Was {profileSize}");
+                            }
+                            else
+                            {
+                                using var iccBuffer = profileSize <= 256
+                                    ? new BufferScope(stackalloc byte[profileSize])
+                                    : new BufferScope(profileSize);
 
-                            reader.GetBytes(iccBuffer.Span);
+                                reader.GetBytes(iccBuffer.Span);
 
-                            var iccDirectory = new IccReader().Extract(iccBuffer.Span);
-                            iccDirectory.Parent = directory;
-                            directories.Add(iccDirectory);
+                                var iccDirectory = new IccReader().Extract(iccBuffer.Span);
+                                iccDirectory.Parent = directory;
+                                directories.Add(iccDirectory);
+                            }
                         }
                     }
                     else
@@ -369,7 +376,7 @@ private static void ReadBitmapHeader(SequentialReader reader, BmpHeaderDirectory
                 }
                 else
                 {
-                    directory.AddError("Unexpected DIB header size: " + headerSize);
+                    directory.AddError($"Unexpected DIB header size: {headerSize}");
                 }
             }
             catch (IOException)

diff --git a/MetadataExtractor/IO/BufferReader.Indexed.cs b/MetadataExtractor/IO/BufferReader.Indexed.cs
@@ -177,6 +177,9 @@ public readonly double GetDouble64(int index)
 
     public readonly string GetString(int index, int bytesRequested, Encoding encoding)
     {
+        if (bytesRequested < 0)
+            throw new ArgumentOutOfRangeException(nameof(bytesRequested), "Must be 0 or greater");
+
         // This check is important on .NET Framework
         if (bytesRequested is 0)
         {

diff --git a/MetadataExtractor/IO/BufferReader.Sequential.cs b/MetadataExtractor/IO/BufferReader.Sequential.cs
@@ -119,17 +119,20 @@ public ulong GetUInt64()
 
     public string GetString(int bytesRequested, Encoding encoding)
     {
+        if (bytesRequested < 0)
+            throw new ArgumentOutOfRangeException(nameof(bytesRequested), "Must be 0 or greater");
+
         // This check is important on .NET Framework
         if (bytesRequested is 0)
             return "";
 
-        Span<byte> bytes = bytesRequested <= 256
-            ? stackalloc byte[bytesRequested]
-            : new byte[bytesRequested];
+        using BufferScope bytes = bytesRequested <= 256
+            ? new BufferScope(stackalloc byte[bytesRequested])
+            : new BufferScope(bytesRequested);
 
-        GetBytes(bytes);
+        GetBytes(bytes.Span);
 
-        return encoding.GetString(bytes);
+        return encoding.GetString(bytes.Span);
     }
 
     public StringValue GetNullTerminatedStringValue(int maxLengthBytes, Encoding? encoding = null, bool moveToMaxLength = false)

diff --git a/MetadataExtractor/IO/IndexedReader.cs b/MetadataExtractor/IO/IndexedReader.cs
@@ -320,6 +320,9 @@ public double GetDouble64(int index)
         /// <exception cref="IOException"/>
         public string GetString(int index, int bytesRequested, Encoding encoding)
         {
+            if (bytesRequested < 0)
+                throw new ArgumentOutOfRangeException(nameof(bytesRequested), "Must be 0 or greater");
+
             // This check is important on .NET Framework
             if (bytesRequested is 0)
             {