[PM-36584] Bidirectional C2 in icons.bitwarden.net (#7668)

* adds png chunk verification

* changes from fail open to fail closed when png/ico are malformed

* removes unused const

* fixes tests for stripped png

* adds maxEntries for ico

* removes redundant count == 0 check

Author: jengstrom-bw

src/Icons/Icons.csproj

@@ -22,4 +22,8 @@
     <ProjectReference Include="..\Core\Core.csproj" />
   </ItemGroup>
 
+  <ItemGroup>
+    <InternalsVisibleTo Include="Icons.Test" />
+  </ItemGroup>
+
 </Project>

src/Icons/Models/IconLink.cs

@@ -1,5 +1,6 @@
 #nullable enable
 
+using System.Buffers.Binary;
 using System.Text;
 using AngleSharp.Dom;
 using Bit.Icons.Extensions;
@@ -13,25 +14,28 @@ public class IconLink
     private static readonly HashSet<string> _blocklistedRels = new(StringComparer.InvariantCultureIgnoreCase) { "preload", "image_src", "preconnect", "canonical", "alternate", "stylesheet" };
     private static readonly HashSet<string> _iconExtensions = new(StringComparer.InvariantCultureIgnoreCase) { ".ico", ".png", ".jpg", ".jpeg" };
     private const string _pngMediaType = "image/png";
-    private static readonly byte[] _pngHeader = new byte[] { 137, 80, 78, 71 };
+    private static readonly byte[] _pngHeader = [137, 80, 78, 71];
+    private static readonly byte[] _pngSignature = [137, 80, 78, 71, 13, 10, 26, 10];
     private static readonly byte[] _webpHeader = Encoding.UTF8.GetBytes("RIFF");
 
+    private static readonly HashSet<string> _allowedPngChunkTypes = new(StringComparer.Ordinal)
+    {
+        "IHDR", "PLTE", "IDAT", "IEND", "tRNS", "sRGB", "gAMA", "cHRM",
+    };
+
     private const string _icoMediaType = "image/x-icon";
     private const string _icoAltMediaType = "image/vnd.microsoft.icon";
     private static readonly byte[] _icoHeader = new byte[] { 00, 00, 01, 00 };
 
     private const string _jpegMediaType = "image/jpeg";
     private static readonly byte[] _jpegHeader = new byte[] { 255, 216, 255 };
 
-    private const string _svgXmlMediaType = "image/svg+xml";
-
     private static readonly HashSet<string> _allowedMediaTypes = new(StringComparer.InvariantCultureIgnoreCase)
     {
         _pngMediaType,
         _icoMediaType,
         _icoAltMediaType,
         _jpegMediaType,
-        _svgXmlMediaType,
     };
 
     private bool _useUriDirectly = false;
@@ -156,6 +160,20 @@ public bool IsUsable()
             return null;
         }
 
+        if (HeaderMatch(bytes, _pngHeader))
+        {
+            bytes = StripPngMetadata(bytes);
+        }
+        else if (HeaderMatch(bytes, _icoHeader))
+        {
+            bytes = StripIcoEmbeddedPngMetadata(bytes);
+        }
+
+        if (bytes == null)
+        {
+            return null;
+        }
+
         return new Icon { Image = bytes, Format = format };
     }
 
@@ -217,4 +235,126 @@ private static string DetermineImageFormatFromFile(byte[] imageBytes)
             return string.Empty;
         }
     }
+
+    internal static byte[]? StripPngMetadata(byte[] bytes)
+    {
+        if (bytes.Length < _pngSignature.Length || !HeaderMatch(bytes, _pngSignature))
+        {
+            return null;
+        }
+
+        using var output = new MemoryStream(bytes.Length);
+        output.Write(bytes, 0, _pngSignature.Length);
+
+        var offset = _pngSignature.Length;
+        var seenIend = false;
+        while (offset + 12 <= bytes.Length)
+        {
+            var length = BinaryPrimitives.ReadInt32BigEndian(bytes.AsSpan(offset, 4));
+            if (length < 0 || (long)offset + 12 + length > bytes.Length)
+            {
+                return null;
+            }
+
+            var type = Encoding.ASCII.GetString(bytes, offset + 4, 4);
+            var chunkSize = 12 + length;
+
+            if (_allowedPngChunkTypes.Contains(type))
+            {
+                output.Write(bytes, offset, chunkSize);
+            }
+
+            offset += chunkSize;
+
+            if (type == "IEND")
+            {
+                seenIend = true;
+                break;
+            }
+        }
+
+        if (!seenIend)
+        {
+            return null;
+        }
+
+        return output.ToArray();
+    }
+
+    internal static byte[]? StripIcoEmbeddedPngMetadata(byte[] bytes)
+    {
+        const int dirHeaderSize = 6;
+        const int entrySize = 16;
+
+        if (bytes.Length < dirHeaderSize || !HeaderMatch(bytes, _icoHeader))
+        {
+            return null;
+        }
+
+        const int maxEntries = 64;
+        var count = BinaryPrimitives.ReadUInt16LittleEndian(bytes.AsSpan(4, 2));
+        if (count == 0 || count > maxEntries)
+        {
+            return null;
+        }
+
+        var directorySize = dirHeaderSize + count * entrySize;
+        if (bytes.Length < directorySize)
+        {
+            return null;
+        }
+
+        var entries = new byte[count][];
+        var images = new byte[count][];
+
+        for (var i = 0; i < count; i++)
+        {
+            var entryOffset = dirHeaderSize + i * entrySize;
+            var size = BinaryPrimitives.ReadInt32LittleEndian(bytes.AsSpan(entryOffset + 8, 4));
+            var dataOffset = BinaryPrimitives.ReadInt32LittleEndian(bytes.AsSpan(entryOffset + 12, 4));
+
+            if (size < 0 || dataOffset < directorySize || (long)dataOffset + size > bytes.Length)
+            {
+                return null;
+            }
+
+            var image = new byte[size];
+            Buffer.BlockCopy(bytes, dataOffset, image, 0, size);
+
+            if (HeaderMatch(image, _pngHeader))
+            {
+                var stripped = StripPngMetadata(image);
+                if (stripped == null)
+                {
+                    return null;
+                }
+                image = stripped;
+            }
+
+            var entry = new byte[entrySize];
+            Buffer.BlockCopy(bytes, entryOffset, entry, 0, entrySize);
+
+            entries[i] = entry;
+            images[i] = image;
+        }
+
+        using var output = new MemoryStream(bytes.Length);
+        output.Write(bytes, 0, dirHeaderSize);
+
+        var imageOffset = directorySize;
+        for (var i = 0; i < count; i++)
+        {
+            BinaryPrimitives.WriteInt32LittleEndian(entries[i].AsSpan(8, 4), images[i].Length);
+            BinaryPrimitives.WriteInt32LittleEndian(entries[i].AsSpan(12, 4), imageOffset);
+            output.Write(entries[i], 0, entrySize);
+            imageOffset += images[i].Length;
+        }
+
+        for (var i = 0; i < count; i++)
+        {
+            output.Write(images[i], 0, images[i].Length);
+        }
+
+        return output.ToArray();
+    }
 }

test/Icons.Test/Models/IconHttpResponseTests.cs

@@ -93,7 +93,11 @@ private static IHttpClientFactory UsableIconHttpClientFactory()
         var handler = new MockedHttpMessageHandler();
         handler.Fallback
             .WithStatusCode(HttpStatusCode.OK)
-            .WithContent("image/png", new byte[] { 137, 80, 78, 71 });
+            .WithContent("image/png", new byte[]
+            {
+                137, 80, 78, 71, 13, 10, 26, 10,
+                0, 0, 0, 0, 73, 69, 78, 68, 0, 0, 0, 0,
+            });
 
         substitute.CreateClient("Icons").Returns(handler.ToHttpClient());
         return substitute;