diff --git a/Jellyfin.sln b/Jellyfin.sln index b0d5a5eb47..b666e4ae16 100644 --- a/Jellyfin.sln +++ b/Jellyfin.sln @@ -99,6 +99,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Jellyfin.Database.Implement EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Jellyfin.CodeAnalysis", "src\Jellyfin.CodeAnalysis\Jellyfin.CodeAnalysis.csproj", "{11643D0F-6761-4EF7-AB71-6F9F8DE00714}" EndProject +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Jellyfin.Drawing.Skia.Tests", "tests\Jellyfin.Drawing.Skia.Tests\Jellyfin.Drawing.Skia.Tests.csproj", "{E24A279C-9A37-419A-8F9C-853C11FBE753}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -265,6 +267,10 @@ Global {11643D0F-6761-4EF7-AB71-6F9F8DE00714}.Debug|Any CPU.Build.0 = Debug|Any CPU {11643D0F-6761-4EF7-AB71-6F9F8DE00714}.Release|Any CPU.ActiveCfg = Release|Any CPU {11643D0F-6761-4EF7-AB71-6F9F8DE00714}.Release|Any CPU.Build.0 = Release|Any CPU + {E24A279C-9A37-419A-8F9C-853C11FBE753}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {E24A279C-9A37-419A-8F9C-853C11FBE753}.Debug|Any CPU.Build.0 = Debug|Any CPU + {E24A279C-9A37-419A-8F9C-853C11FBE753}.Release|Any CPU.ActiveCfg = Release|Any CPU + {E24A279C-9A37-419A-8F9C-853C11FBE753}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -297,6 +303,7 @@ Global {A5590358-33CC-4B39-BDE7-DC62FEB03C76} = {4C54CE05-69C8-48FA-8785-39F7F6DB1CAD} {8C9F9221-8415-496C-B1F5-E7756F03FA59} = {4C54CE05-69C8-48FA-8785-39F7F6DB1CAD} {11643D0F-6761-4EF7-AB71-6F9F8DE00714} = {C9F0AB5D-F4D7-40C8-A353-3305C86D6D4C} + {E24A279C-9A37-419A-8F9C-853C11FBE753} = {FBBB5129-006E-4AD7-BAD5-8B7CA1D10ED6} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {3448830C-EBDC-426C-85CD-7BBB9651A7FE} diff --git a/src/Jellyfin.Drawing.Skia/SkiaEncoder.cs b/src/Jellyfin.Drawing.Skia/SkiaEncoder.cs index b6d2914efa..3e353db8de 100644 --- a/src/Jellyfin.Drawing.Skia/SkiaEncoder.cs +++ b/src/Jellyfin.Drawing.Skia/SkiaEncoder.cs @@ -11,6 +11,7 @@ using MediaBrowser.Controller.Drawing; using MediaBrowser.Model.Drawing; using Microsoft.Extensions.Logging; using SkiaSharp; +using Svg; using Svg.Skia; namespace Jellyfin.Drawing.Skia; @@ -48,6 +49,13 @@ public class SkiaEncoder : IImageEncoder /// public static readonly SKSamplingOptions DefaultSamplingOptions = new SKSamplingOptions(SKFilterMode.Linear, SKMipmapMode.Linear); + static SkiaEncoder() + { + SvgDocument.ResolveExternalElements = ExternalType.None; + SvgDocument.ResolveExternalImages = ExternalType.None; + SvgDocument.ResolveExternalXmlEntites = ExternalType.None; + } + /// /// Initializes a new instance of the class. /// @@ -183,6 +191,12 @@ public class SkiaEncoder : IImageEncoder var extension = Path.GetExtension(path.AsSpan()); if (extension.Equals(".svg", StringComparison.OrdinalIgnoreCase)) { + if (!SvgSecurityValidator.IsSafe(path, out var reason)) + { + _logger.LogError("Refusing to determine dimensions for SVG {FilePath}: {Reason}", path, reason); + return default; + } + using var svg = new SKSvg(); try { @@ -445,6 +459,12 @@ public class SkiaEncoder : IImageEncoder throw new FileNotFoundException("File not found", path); } + if (!SvgSecurityValidator.IsSafe(path, out var reason)) + { + _logger.LogError("Refusing to render SVG {FilePath}: {Reason}", path, reason); + return null; + } + using var svg = SKSvg.CreateFromFile(path); if (svg.Drawable is null) { diff --git a/src/Jellyfin.Drawing.Skia/SvgSecurityValidator.cs b/src/Jellyfin.Drawing.Skia/SvgSecurityValidator.cs new file mode 100644 index 0000000000..f8a1d7d443 --- /dev/null +++ b/src/Jellyfin.Drawing.Skia/SvgSecurityValidator.cs @@ -0,0 +1,339 @@ +using System; +using System.Buffers; +using System.Diagnostics.CodeAnalysis; +using System.IO; +using System.IO.Compression; +using System.Runtime.CompilerServices; +using System.Text; +using System.Xml; + +[assembly: InternalsVisibleTo("Jellyfin.Drawing.Skia.Tests")] + +namespace Jellyfin.Drawing.Skia; + +/// +/// Validates that an SVG document does not reference external resources before it is rasterized. +/// +internal static class SvgSecurityValidator +{ + // Guards against a chain of nested data:image/svg+xml payloads. + private const int MaxDataUriDepth = 4; + + // Upper bound for a decompressed svgz payload carried inside a data URI, to guard against decompression bombs. + private const int MaxDecompressedBytes = 16 * 1024 * 1024; + + private const int DecompressBufferSize = 81920; + + private static readonly XmlReaderSettings _scanSettings = new() + { + DtdProcessing = DtdProcessing.Parse, + XmlResolver = null, + MaxCharactersFromEntities = 1024 * 1024, + IgnoreComments = true, + IgnoreProcessingInstructions = true, + IgnoreWhitespace = true, + CloseInput = false + }; + + /// + /// Determines whether the SVG at the given path is safe to rasterize, i.e. contains no references + /// to external resources. + /// + /// The path to the SVG file. + /// When this method returns false, the reason the document was rejected. + /// true if the document is free of external references; otherwise false. + public static bool IsSafe(string path, [NotNullWhen(false)] out string? reason) + { + try + { + using var stream = File.OpenRead(path); + reason = Validate(stream, 0); + } + catch (IOException ex) + { + reason = "Unable to read the file for validation: " + ex.Message; + } + catch (UnauthorizedAccessException ex) + { + reason = "Unable to read the file for validation: " + ex.Message; + } + + return reason is null; + } + + /// + /// Determines whether the SVG in the given stream is safe to rasterize. + /// + /// The stream containing the SVG document. + /// When this method returns false, the reason the document was rejected. + /// true if the document is free of external references; otherwise false. + public static bool IsSafe(Stream stream, [NotNullWhen(false)] out string? reason) + { + reason = Validate(stream, 0); + return reason is null; + } + + private static string? Validate(Stream stream, int depth) + { + try + { + using var reader = XmlReader.Create(stream, _scanSettings); + while (reader.Read()) + { + switch (reader.NodeType) + { + case XmlNodeType.DocumentType: + { + var subset = reader.Value; + if (!string.IsNullOrEmpty(subset) + && (subset.Contains("SYSTEM", StringComparison.OrdinalIgnoreCase) + || subset.Contains("PUBLIC", StringComparison.OrdinalIgnoreCase))) + { + return "The document declares an external DTD entity"; + } + + break; + } + + case XmlNodeType.Element when reader.HasAttributes: + { + for (var i = 0; i < reader.AttributeCount; i++) + { + reader.MoveToAttribute(i); + var isHref = reader.LocalName.Equals("href", StringComparison.OrdinalIgnoreCase); + var reason = isHref + ? ValidateReference(reader.Value, depth, "href") + : ValidateCss(reader.Value, depth); + if (reason is not null) + { + return reason; + } + } + + reader.MoveToElement(); + break; + } + + case XmlNodeType.Text: + case XmlNodeType.CDATA: + { + var reason = ValidateCss(reader.Value, depth); + if (reason is not null) + { + return reason; + } + + break; + } + } + } + + return null; + } + catch (XmlException ex) + { + // Malformed markup, a forbidden DTD construct or an unresolved external entity: refuse to render. + return "The document could not be safely parsed: " + ex.Message; + } + } + + private static string? ValidateReference(ReadOnlySpan value, int depth, string context) + { + var trimmed = value.Trim(); + if (trimmed.IsEmpty || trimmed[0] == '#') + { + return null; + } + + if (trimmed.StartsWith("data:", StringComparison.OrdinalIgnoreCase)) + { + return ValidateDataUri(trimmed, depth, context); + } + + return "An external resource is referenced via " + context; + } + + private static string? ValidateDataUri(ReadOnlySpan dataUri, int depth, string context) + { + // "data:[][;base64]," (mirrors Svg.Model's data URI parsing). + var comma = dataUri.IndexOf(','); + if (comma < 0) + { + return "A malformed data URI is referenced via " + context; + } + + var header = dataUri[5..comma]; + var firstSeparator = header.IndexOf(';'); + var mediaType = (firstSeparator < 0 ? header : header[..firstSeparator]).Trim(); + + // Only "image/svg+xml" is re-parsed as SVG by the renderer; any other type is treated as raster data. + if (!mediaType.Contains('/') || !mediaType.Equals("image/svg+xml", StringComparison.OrdinalIgnoreCase)) + { + return null; + } + + if (depth >= MaxDataUriDepth) + { + return "Nested data URIs exceed the allowed depth"; + } + + var lastSeparator = header.LastIndexOf(';'); + var isBase64 = lastSeparator >= 0 + && header[(lastSeparator + 1)..].Trim().Equals("base64", StringComparison.OrdinalIgnoreCase); + + var payload = dataUri[(comma + 1)..].Trim(); + byte[]? buffer = null; + try + { + int length; + if (isBase64) + { + buffer = ArrayPool.Shared.Rent((payload.Length / 4 * 3) + 3); + if (!Convert.TryFromBase64Chars(payload, buffer, out length)) + { + return "An undecodable data URI is referenced via " + context; + } + } + else + { + var unescaped = Uri.UnescapeDataString(payload.ToString()); + buffer = ArrayPool.Shared.Rent(Encoding.UTF8.GetMaxByteCount(unescaped.Length)); + length = Encoding.UTF8.GetBytes(unescaped, buffer); + } + + if (length > 2 && buffer[0] == 0x1F && buffer[1] == 0x8B) + { + using var decompressed = Decompress(buffer, length); + return Validate(decompressed, depth + 1); + } + + using var stream = new MemoryStream(buffer, 0, length, false); + return Validate(stream, depth + 1); + } + catch (FormatException ex) + { + return "An undecodable data URI is referenced via " + context + ": " + ex.Message; + } + catch (InvalidDataException ex) + { + return "An invalid compressed data URI is referenced via " + context + ": " + ex.Message; + } + finally + { + if (buffer is not null) + { + ArrayPool.Shared.Return(buffer); + } + } + } + + private static MemoryStream Decompress(byte[] compressed, int length) + { + using var input = new MemoryStream(compressed, 0, length, false); + using var gzip = new GZipStream(input, CompressionMode.Decompress); + var output = new MemoryStream(); + var buffer = ArrayPool.Shared.Rent(DecompressBufferSize); + try + { + var total = 0; + int read; + while ((read = gzip.Read(buffer, 0, buffer.Length)) > 0) + { + total += read; + if (total > MaxDecompressedBytes) + { + throw new InvalidDataException("Compressed data URI exceeds the allowed size"); + } + + output.Write(buffer, 0, read); + } + } + catch + { + output.Dispose(); + throw; + } + finally + { + ArrayPool.Shared.Return(buffer); + } + + output.Position = 0; + return output; + } + + private static string? ValidateCss(ReadOnlySpan value, int depth) + { + if (value.IsEmpty) + { + return null; + } + + var index = 0; + while (true) + { + var found = value[index..].IndexOf("url(", StringComparison.OrdinalIgnoreCase); + if (found < 0) + { + break; + } + + var start = index + found + 4; + var close = value[start..].IndexOf(')'); + if (close < 0) + { + break; + } + + var target = value.Slice(start, close).Trim(); + target = target.Trim('\''); + target = target.Trim('"').Trim(); + var reason = ValidateReference(target, depth, "url()"); + if (reason is not null) + { + return reason; + } + + index = start + close + 1; + if (index >= value.Length) + { + break; + } + } + + // Handle the bare "@import '...';" form (the "@import url(...)" form is covered above). + index = 0; + while (true) + { + var found = value[index..].IndexOf("@import", StringComparison.OrdinalIgnoreCase); + if (found < 0) + { + break; + } + + var rest = value[(index + found + 7)..]; + var quote = rest.IndexOfAny('\'', '"'); + if (quote >= 0) + { + var afterQuote = rest[(quote + 1)..]; + var end = afterQuote.IndexOfAny('\'', '"'); + if (end >= 0) + { + var reason = ValidateReference(afterQuote[..end], depth, "@import"); + if (reason is not null) + { + return reason; + } + } + } + + index = index + found + 7; + if (index >= value.Length) + { + break; + } + } + + return null; + } +} diff --git a/tests/Jellyfin.Drawing.Skia.Tests/Jellyfin.Drawing.Skia.Tests.csproj b/tests/Jellyfin.Drawing.Skia.Tests/Jellyfin.Drawing.Skia.Tests.csproj new file mode 100644 index 0000000000..b6dc5dfb92 --- /dev/null +++ b/tests/Jellyfin.Drawing.Skia.Tests/Jellyfin.Drawing.Skia.Tests.csproj @@ -0,0 +1,26 @@ + + + + + {E24A279C-9A37-419A-8F9C-853C11FBE753} + Exe + + + + + + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + + + + + + + + diff --git a/tests/Jellyfin.Drawing.Skia.Tests/SvgSecurityValidatorTests.cs b/tests/Jellyfin.Drawing.Skia.Tests/SvgSecurityValidatorTests.cs new file mode 100644 index 0000000000..30b7983ece --- /dev/null +++ b/tests/Jellyfin.Drawing.Skia.Tests/SvgSecurityValidatorTests.cs @@ -0,0 +1,99 @@ +using System.IO; +using Xunit; + +namespace Jellyfin.Drawing.Skia.Tests; + +public static class SvgSecurityValidatorTests +{ + public static TheoryData ExternalReferenceSvgs => new() + { + // SSRF via (xlink:href and plain href) + "", + "", + // Local file disclosure + "", + // Memory exhaustion DoS + "", + // external reference + "", + // CSS url() external reference in an attribute + "", + // @import in a style block + "", + // Relative path traversal (resolves against the document location -> local file read) + "", + // XXE via external entity + "]>&xxe;", + // Entity-expansion (billion laughs) denial of service + "]>&f;", + // Nested SVG in a base64 data: URI whose inner document references an external resource + "", + // Nested SVG in a URL-encoded (non-base64) data: URI referencing an external resource + "", + // Nested gzip-compressed (svgz) data: URI whose inner document references an external resource + "", + }; + + public static TheoryData SafeSvgs => new() + { + "", + // Same-document fragment references are allowed + "", + // Inline data URIs are allowed + "", + // A DOCTYPE without external entities is allowed + "", + // An internal general entity with no external reference is allowed (and is expanded by the renderer) + "]>", + // A nested data:image/svg+xml payload that is itself self-contained is allowed + "", + // A self-contained gzip-compressed (svgz) data: URI is allowed + "", + }; + + [Theory] + [MemberData(nameof(ExternalReferenceSvgs))] + public static void IsSafe_ExternalReference_ReturnsFalse(string svg) + { + var path = WriteTemp(svg); + try + { + Assert.False(SvgSecurityValidator.IsSafe(path, out var reason)); + Assert.NotNull(reason); + } + finally + { + File.Delete(path); + } + } + + [Theory] + [MemberData(nameof(SafeSvgs))] + public static void IsSafe_NoExternalReference_ReturnsTrue(string svg) + { + var path = WriteTemp(svg); + try + { + Assert.True(SvgSecurityValidator.IsSafe(path, out var reason)); + Assert.Null(reason); + } + finally + { + File.Delete(path); + } + } + + [Fact] + public static void IsSafe_MissingFile_ReturnsFalse() + { + Assert.False(SvgSecurityValidator.IsSafe(Path.Combine(Path.GetTempPath(), "does-not-exist-" + Path.GetRandomFileName() + ".svg"), out var reason)); + Assert.NotNull(reason); + } + + private static string WriteTemp(string svg) + { + var path = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName() + ".svg"); + File.WriteAllText(path, svg); + return path; + } +}