diff --git a/src/Icons/Icons.csproj b/src/Icons/Icons.csproj index 65aaebed9618..c217eb32b548 100644 --- a/src/Icons/Icons.csproj +++ b/src/Icons/Icons.csproj @@ -22,4 +22,8 @@ + + + + diff --git a/src/Icons/Models/IconLink.cs b/src/Icons/Models/IconLink.cs index 8c09058bb2d2..b1f38af7a3ff 100644 --- a/src/Icons/Models/IconLink.cs +++ b/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,9 +14,15 @@ public class IconLink private static readonly HashSet _blocklistedRels = new(StringComparer.InvariantCultureIgnoreCase) { "preload", "image_src", "preconnect", "canonical", "alternate", "stylesheet" }; private static readonly HashSet _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 _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 }; @@ -23,15 +30,12 @@ public class IconLink 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 _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(); + } } diff --git a/test/Icons.Test/Models/IconHttpResponseTests.cs b/test/Icons.Test/Models/IconHttpResponseTests.cs index d6c792e2e782..77e12430c937 100644 --- a/test/Icons.Test/Models/IconHttpResponseTests.cs +++ b/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; diff --git a/test/Icons.Test/Models/IconLinkTests.cs b/test/Icons.Test/Models/IconLinkTests.cs index a4087f2b7428..8683db0cbde5 100644 --- a/test/Icons.Test/Models/IconLinkTests.cs +++ b/test/Icons.Test/Models/IconLinkTests.cs @@ -1,4 +1,6 @@ -using System.Net; +using System.Buffers.Binary; +using System.Net; +using System.Text; using AngleSharp.Dom; using Bit.Icons.Models; using Bit.Icons.Services; @@ -10,6 +12,8 @@ namespace Bit.Icons.Test.Models; public class IconLinkTests { + private static readonly byte[] _pngSignature = new byte[] { 137, 80, 78, 71, 13, 10, 26, 10 }; + private readonly IElement _element; private readonly Uri _uri = new("https://icon.test"); private readonly ILogger _logger = Substitute.For>(); @@ -75,6 +79,227 @@ public async Task FetchAsync_Unvalidated_ReturnsNull() Assert.Null(result); } + [Fact] + public void StripPngMetadata_RemovesDisallowedChunks_KeepsAllowedChunks() + { + var png = BuildPng(new[] + { + ("IHDR", new byte[13]), + ("tEXt", Encoding.ASCII.GetBytes("Comment\0secret")), + ("sRGB", new byte[] { 0 }), + ("iTXt", Encoding.ASCII.GetBytes("evil")), + ("IDAT", new byte[] { 1, 2, 3 }), + ("zTXt", Encoding.ASCII.GetBytes("evil")), + ("IEND", Array.Empty()), + }); + + var stripped = IconLink.StripPngMetadata(png); + var chunkTypes = ReadChunkTypes(stripped); + + Assert.Equal(new[] { "IHDR", "sRGB", "IDAT", "IEND" }, chunkTypes); + Assert.DoesNotContain("tEXt", chunkTypes); + Assert.DoesNotContain("iTXt", chunkTypes); + Assert.DoesNotContain("zTXt", chunkTypes); + } + + [Fact] + public void StripPngMetadata_StopsAtIend_DropsTrailingGarbage() + { + var png = BuildPng(new[] + { + ("IHDR", new byte[13]), + ("IDAT", new byte[] { 9, 9 }), + ("IEND", Array.Empty()), + }); + var withTrailer = png.Concat(Encoding.ASCII.GetBytes("trailing-garbage")).ToArray(); + + var stripped = IconLink.StripPngMetadata(withTrailer); + + Assert.Equal(png, stripped); + } + + [Fact] + public void StripPngMetadata_InvalidSignature_ReturnsNull() + { + var bytes = new byte[] { 1, 2, 3, 4, 5 }; + + var stripped = IconLink.StripPngMetadata(bytes); + + Assert.Null(stripped); + } + + [Fact] + public void StripPngMetadata_ChunkLengthOverflowsBuffer_ReturnsNull() + { + var prefix = _pngSignature.ToList(); + // Declare a chunk with absurdly large length but include the 12 bytes + // required for the parser to enter the chunk-walk loop. + prefix.AddRange(new byte[] { 0x7F, 0xFF, 0xFF, 0xFF }); + prefix.AddRange(Encoding.ASCII.GetBytes("IHDR")); + prefix.AddRange(new byte[] { 0, 0, 0, 0 }); + var bytes = prefix.ToArray(); + + var stripped = IconLink.StripPngMetadata(bytes); + + Assert.Null(stripped); + } + + [Fact] + public void StripPngMetadata_MissingIend_ReturnsNull() + { + var png = BuildPng(new[] + { + ("IHDR", new byte[13]), + ("IDAT", new byte[] { 1, 2, 3 }), + }); + + var stripped = IconLink.StripPngMetadata(png); + + Assert.Null(stripped); + } + + [Fact] + public void StripIcoEmbeddedPngMetadata_StripsPngEntries_RewritesOffsetsAndSizes() + { + var pngWithMetadata = BuildPng(new[] + { + ("IHDR", new byte[13]), + ("tEXt", Encoding.ASCII.GetBytes("c\0attacker")), + ("IDAT", new byte[] { 1, 2, 3 }), + ("IEND", Array.Empty()), + }); + var bmp = new byte[] { 9, 8, 7, 6, 5, 4, 3, 2 }; + + var ico = BuildIco(new[] { pngWithMetadata, bmp }); + + var stripped = IconLink.StripIcoEmbeddedPngMetadata(ico); + var entries = ReadIcoEntries(stripped); + + Assert.Equal(2, entries.Count); + // First entry: PNG, metadata removed. + var firstData = stripped.AsSpan(entries[0].Offset, entries[0].Size).ToArray(); + Assert.Equal(new[] { "IHDR", "IDAT", "IEND" }, ReadChunkTypes(firstData)); + Assert.True(firstData.Length < pngWithMetadata.Length); + // Second entry: untouched BMP bytes. + var secondData = stripped.AsSpan(entries[1].Offset, entries[1].Size).ToArray(); + Assert.Equal(bmp, secondData); + // Offsets must point past the directory and pack contiguously. + Assert.Equal(6 + 16 * 2, entries[0].Offset); + Assert.Equal(entries[0].Offset + entries[0].Size, entries[1].Offset); + } + + [Fact] + public void StripIcoEmbeddedPngMetadata_NoPngEntries_PreservesData() + { + var bmpA = new byte[] { 1, 2, 3, 4 }; + var bmpB = new byte[] { 5, 6, 7, 8, 9 }; + + var ico = BuildIco(new[] { bmpA, bmpB }); + var stripped = IconLink.StripIcoEmbeddedPngMetadata(ico); + var entries = ReadIcoEntries(stripped); + + Assert.Equal(bmpA, stripped.AsSpan(entries[0].Offset, entries[0].Size).ToArray()); + Assert.Equal(bmpB, stripped.AsSpan(entries[1].Offset, entries[1].Size).ToArray()); + } + + [Fact] + public void StripIcoEmbeddedPngMetadata_CorruptOffset_ReturnsNull() + { + var ico = BuildIco(new[] { new byte[] { 1, 2, 3, 4 } }); + // Point the entry data offset past end of buffer. + BinaryPrimitives.WriteInt32LittleEndian(ico.AsSpan(6 + 12, 4), ico.Length + 1000); + + var stripped = IconLink.StripIcoEmbeddedPngMetadata(ico); + + Assert.Null(stripped); + } + + [Fact] + public void StripIcoEmbeddedPngMetadata_MalformedEmbeddedPng_ReturnsNull() + { + // Embed bytes that match the 4-byte PNG header but lack a valid signature. + var fakePng = new byte[] { 137, 80, 78, 71, 0, 0, 0, 0 }; + + var ico = BuildIco(new[] { fakePng }); + + var stripped = IconLink.StripIcoEmbeddedPngMetadata(ico); + + Assert.Null(stripped); + } + + private static byte[] BuildPng(IEnumerable<(string Type, byte[] Data)> chunks) + { + using var stream = new MemoryStream(); + stream.Write(_pngSignature); + foreach (var (type, data) in chunks) + { + var lengthBytes = new byte[4]; + BinaryPrimitives.WriteInt32BigEndian(lengthBytes, data.Length); + stream.Write(lengthBytes); + stream.Write(Encoding.ASCII.GetBytes(type)); + stream.Write(data); + // CRC placeholder — strip logic does not validate CRCs. + stream.Write(new byte[] { 0, 0, 0, 0 }); + } + return stream.ToArray(); + } + + private static List ReadChunkTypes(byte[] png) + { + var types = new List(); + var offset = _pngSignature.Length; + while (offset + 12 <= png.Length) + { + var length = BinaryPrimitives.ReadInt32BigEndian(png.AsSpan(offset, 4)); + types.Add(Encoding.ASCII.GetString(png, offset + 4, 4)); + offset += 12 + length; + } + return types; + } + + private static byte[] BuildIco(IReadOnlyList images) + { + const int dirHeaderSize = 6; + const int entrySize = 16; + + using var stream = new MemoryStream(); + stream.Write(new byte[] { 0, 0, 1, 0 }); // reserved, type=icon + var countBytes = new byte[2]; + BinaryPrimitives.WriteUInt16LittleEndian(countBytes, (ushort)images.Count); + stream.Write(countBytes); + + var dataOffset = dirHeaderSize + entrySize * images.Count; + foreach (var image in images) + { + var entry = new byte[entrySize]; + BinaryPrimitives.WriteInt32LittleEndian(entry.AsSpan(8, 4), image.Length); + BinaryPrimitives.WriteInt32LittleEndian(entry.AsSpan(12, 4), dataOffset); + stream.Write(entry); + dataOffset += image.Length; + } + foreach (var image in images) + { + stream.Write(image); + } + return stream.ToArray(); + } + + private static List<(int Size, int Offset)> ReadIcoEntries(byte[] ico) + { + const int dirHeaderSize = 6; + const int entrySize = 16; + var count = BinaryPrimitives.ReadUInt16LittleEndian(ico.AsSpan(4, 2)); + var entries = new List<(int Size, int Offset)>(count); + for (var i = 0; i < count; i++) + { + var entryOffset = dirHeaderSize + i * entrySize; + var size = BinaryPrimitives.ReadInt32LittleEndian(ico.AsSpan(entryOffset + 8, 4)); + var dataOffset = BinaryPrimitives.ReadInt32LittleEndian(ico.AsSpan(entryOffset + 12, 4)); + entries.Add((size, dataOffset)); + } + return entries; + } + private void SetAttributeValue(string attribute, string value) { var attr = Substitute.For();