Prevent SSRF, local file disclosure and DoS via external references in SVG rendering

2026-08-25 11:04:24 +02:00
parent 0d02638e82
commit cefa78fc1d
5 changed files with 450 additions and 0 deletions
@@ -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}
@@ -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
    /// </summary>
    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;
+    }
+
    /// <summary>
    /// Initializes a new instance of the <see cref="SkiaEncoder"/> class.
    /// </summary>
@@ -183,6 +191,12 @@ public class SkiaEncoder : IImageEncoder
        var extension = Path.GetExtension(path.AsSpan());
        if (extension.Equals(".svg", StringComparison.OrdinalIgnoreCase))
        {
+            if (!SvgSecurityValidator.IsSafe(path, _logger))
+            {
+                _logger.LogError("Refusing to determine dimensions for SVG with external references {FilePath}", path);
+                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, _logger))
+        {
+            _logger.LogError("Refusing to render SVG with external references {FilePath}", path);
+            return null;
+        }
+
        using var svg = SKSvg.CreateFromFile(path);
        if (svg.Drawable is null)
        {
@@ -0,0 +1,304 @@
+using System;
+using System.IO;
+using System.IO.Compression;
+using System.Runtime.CompilerServices;
+using System.Text;
+using System.Xml;
+using Microsoft.Extensions.Logging;
+
+[assembly: InternalsVisibleTo("Jellyfin.Drawing.Skia.Tests")]
+
+namespace Jellyfin.Drawing.Skia;
+
+/// <summary>
+/// Validates that an SVG document does not reference external resources before it is rasterized.
+/// </summary>
+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 static readonly XmlReaderSettings _scanSettings = new()
+    {
+        DtdProcessing = DtdProcessing.Parse,
+        XmlResolver = null,
+        MaxCharactersFromEntities = 1024 * 1024,
+        IgnoreComments = true,
+        IgnoreProcessingInstructions = true,
+        IgnoreWhitespace = true,
+        CloseInput = false
+    };
+
+    /// <summary>
+    /// Determines whether the SVG at the given path is safe to rasterize, i.e. contains no references
+    /// to external resources.
+    /// </summary>
+    /// <param name="path">The path to the SVG file.</param>
+    /// <param name="logger">The logger.</param>
+    /// <returns><c>true</c> if the document is free of external references; otherwise <c>false</c>.</returns>
+    public static bool IsSafe(string path, ILogger logger)
+    {
+        try
+        {
+            using var stream = File.OpenRead(path);
+            return IsSafe(stream, logger);
+        }
+        catch (IOException ex)
+        {
+            logger.LogWarning(ex, "Unable to read SVG {Path} for validation, refusing to render", path);
+            return false;
+        }
+        catch (UnauthorizedAccessException ex)
+        {
+            logger.LogWarning(ex, "Unable to read SVG {Path} for validation, refusing to render", path);
+            return false;
+        }
+    }
+
+    /// <summary>
+    /// Determines whether the SVG in the given stream is safe to rasterize.
+    /// </summary>
+    /// <param name="stream">The stream containing the SVG document.</param>
+    /// <param name="logger">The logger.</param>
+    /// <returns><c>true</c> if the document is free of external references; otherwise <c>false</c>.</returns>
+    public static bool IsSafe(Stream stream, ILogger logger)
+        => IsSafe(stream, logger, 0);
+
+    private static bool IsSafe(Stream stream, ILogger logger, 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)))
+                        {
+                            logger.LogWarning("Refusing to render SVG declaring an external DTD entity");
+                            return false;
+                        }
+
+                        break;
+
+                    case XmlNodeType.Element when reader.HasAttributes:
+                        for (var i = 0; i < reader.AttributeCount; i++)
+                        {
+                            reader.MoveToAttribute(i);
+                            var value = reader.Value;
+                            if (reader.LocalName.Equals("href", StringComparison.OrdinalIgnoreCase))
+                            {
+                                if (!IsReferenceSafe(value, logger, depth))
+                                {
+                                    logger.LogWarning("Refusing to render SVG referencing external resource via href");
+                                    return false;
+                                }
+                            }
+                            else if (HasUnsafeCssReference(value, logger, depth))
+                            {
+                                logger.LogWarning("Refusing to render SVG referencing external resource via style/url()");
+                                return false;
+                            }
+                        }
+
+                        reader.MoveToElement();
+                        break;
+
+                    case XmlNodeType.Text:
+                    case XmlNodeType.CDATA:
+                        if (HasUnsafeCssReference(reader.Value, logger, depth))
+                        {
+                            logger.LogWarning("Refusing to render SVG referencing external resource in style block");
+                            return false;
+                        }
+
+                        break;
+                }
+            }
+
+            return true;
+        }
+        catch (XmlException ex)
+        {
+            // Malformed markup, a forbidden DTD construct or an unresolved external entity: refuse to render.
+            logger.LogWarning(ex, "Refusing to render SVG that could not be safely parsed");
+            return false;
+        }
+    }
+
+    private static bool IsReferenceSafe(string? value, ILogger logger, int depth)
+    {
+        if (string.IsNullOrEmpty(value))
+        {
+            return true;
+        }
+
+        var trimmed = value.Trim();
+        if (trimmed.Length == 0 || trimmed[0] == '#')
+        {
+            return true;
+        }
+
+        if (trimmed.StartsWith("data:", StringComparison.OrdinalIgnoreCase))
+        {
+            return IsDataUriSafe(trimmed, logger, depth);
+        }
+
+        return false;
+    }
+
+    private static bool IsDataUriSafe(string dataUri, ILogger logger, int depth)
+    {
+        // "data:[<mediatype>][;base64],<payload>" (mirrors Svg.Model's data URI parsing).
+        var comma = dataUri.IndexOf(',', StringComparison.Ordinal);
+        if (comma < 0)
+        {
+            return false;
+        }
+
+        var header = dataUri[5..comma];
+        var segments = header.Split(';');
+        var mediaType = segments.Length > 0 && segments[0].Contains('/', StringComparison.Ordinal)
+            ? segments[0].Trim()
+            : "text/plain";
+
+        // Only "image/svg+xml" is re-parsed as SVG by the renderer; any other type is treated as raster data.
+        if (!mediaType.Equals("image/svg+xml", StringComparison.OrdinalIgnoreCase))
+        {
+            return true;
+        }
+
+        if (depth >= MaxDataUriDepth)
+        {
+            logger.LogWarning("Refusing to render SVG with nested data URIs exceeding the allowed depth");
+            return false;
+        }
+
+        var isBase64 = segments.Length > 0 && segments[^1].Trim().Equals("base64", StringComparison.OrdinalIgnoreCase);
+        try
+        {
+            var payload = dataUri[(comma + 1)..];
+            byte[] bytes = isBase64
+                ? Convert.FromBase64String(payload.Trim())
+                : Encoding.UTF8.GetBytes(Uri.UnescapeDataString(payload));
+
+            if (bytes.Length > 2 && bytes[0] == 0x1F && bytes[1] == 0x8B)
+            {
+                bytes = Decompress(bytes);
+            }
+
+            using var ms = new MemoryStream(bytes, false);
+            return IsSafe(ms, logger, depth + 1);
+        }
+        catch (FormatException ex)
+        {
+            logger.LogWarning(ex, "Refusing to render SVG with an undecodable data URI");
+            return false;
+        }
+        catch (InvalidDataException ex)
+        {
+            logger.LogWarning(ex, "Refusing to render SVG with an invalid compressed data URI");
+            return false;
+        }
+    }
+
+    private static byte[] Decompress(byte[] compressed)
+    {
+        using var input = new MemoryStream(compressed, false);
+        using var gzip = new GZipStream(input, CompressionMode.Decompress);
+        using var output = new MemoryStream();
+        var buffer = new byte[81920];
+        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);
+        }
+
+        return output.ToArray();
+    }
+
+    private static bool HasUnsafeCssReference(string? value, ILogger logger, int depth)
+    {
+        if (string.IsNullOrEmpty(value))
+        {
+            return false;
+        }
+
+        var span = value.AsSpan();
+        var index = 0;
+        while (true)
+        {
+            var found = span[index..].IndexOf("url(", StringComparison.OrdinalIgnoreCase);
+            if (found < 0)
+            {
+                break;
+            }
+
+            var start = index + found + 4;
+            var close = span[start..].IndexOf(')');
+            if (close < 0)
+            {
+                break;
+            }
+
+            var target = span.Slice(start, close).Trim();
+            target = target.Trim('\'');
+            target = target.Trim('"').Trim();
+            if (!IsReferenceSafe(target.ToString(), logger, depth))
+            {
+                return true;
+            }
+
+            index = start + close + 1;
+            if (index >= span.Length)
+            {
+                break;
+            }
+        }
+
+        // Handle the bare "@import '...';" form (the "@import url(...)" form is covered above).
+        index = 0;
+        while (true)
+        {
+            var found = span[index..].IndexOf("@import", StringComparison.OrdinalIgnoreCase);
+            if (found < 0)
+            {
+                break;
+            }
+
+            var rest = span[(index + found + 7)..];
+            var quote = rest.IndexOfAny('\'', '"');
+            if (quote >= 0)
+            {
+                var afterQuote = rest[(quote + 1)..];
+                var end = afterQuote.IndexOfAny('\'', '"');
+                if (end >= 0 && !IsReferenceSafe(afterQuote[..end].Trim().ToString(), logger, depth))
+                {
+                    return true;
+                }
+            }
+
+            index = index + found + 7;
+            if (index >= span.Length)
+            {
+                break;
+            }
+        }
+
+        return false;
+    }
+}
@@ -0,0 +1,26 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <!-- ProjectGuid is only included as a requirement for SonarQube analysis -->
+  <PropertyGroup>
+    <ProjectGuid>{E24A279C-9A37-419A-8F9C-853C11FBE753}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <PackageReference Include="Microsoft.NET.Test.Sdk" />
+    <PackageReference Include="xunit.v3" />
+    <PackageReference Include="xunit.runner.visualstudio">
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+      <PrivateAssets>all</PrivateAssets>
+    </PackageReference>
+    <PackageReference Include="coverlet.collector">
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+      <PrivateAssets>all</PrivateAssets>
+    </PackageReference>
+  </ItemGroup>
+
+  <ItemGroup>
+    <ProjectReference Include="../../src/Jellyfin.Drawing.Skia/Jellyfin.Drawing.Skia.csproj" />
+  </ItemGroup>
+
+</Project>
@@ -0,0 +1,93 @@
+using System.IO;
+using Microsoft.Extensions.Logging.Abstractions;
+using Xunit;
+
+namespace Jellyfin.Drawing.Skia.Tests;
+
+public static class SvgSecurityValidatorTests
+{
+    public static TheoryData<string> ExternalReferenceSvgs => new()
+    {
+        // SSRF via <image> (xlink:href and plain href)
+        "<svg xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink' width='16' height='16'><image xlink:href='http://169.254.169.254/latest/meta-data/' width='16' height='16'/></svg>",
+        "<svg xmlns='http://www.w3.org/2000/svg' width='16' height='16'><image href='https://example.invalid/a.png' width='16' height='16'/></svg>",
+        // Local file disclosure
+        "<svg xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink' width='16' height='16'><image xlink:href='file:///etc/passwd' width='16' height='16'/></svg>",
+        // Memory exhaustion DoS
+        "<svg xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink' width='16' height='16'><image xlink:href='file:///dev/urandom' width='16' height='16'/></svg>",
+        // <use> external reference
+        "<svg xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink' width='16' height='16'><use xlink:href='http://example.invalid/c.svg#a'/></svg>",
+        // CSS url() external reference in an attribute
+        "<svg xmlns='http://www.w3.org/2000/svg' width='16' height='16'><rect width='16' height='16' style=\"fill:url(http://example.invalid/d.svg#g)\"/></svg>",
+        // @import in a style block
+        "<svg xmlns='http://www.w3.org/2000/svg' width='16' height='16'><style>@import 'http://example.invalid/e.css';</style><rect width='16' height='16'/></svg>",
+        // Relative path traversal (resolves against the document location -> local file read)
+        "<svg xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink' width='16' height='16'><image xlink:href='../../../../etc/hosts' width='16' height='16'/></svg>",
+        // XXE via external entity
+        "<?xml version='1.0'?><!DOCTYPE svg [<!ENTITY xxe SYSTEM 'file:///etc/passwd'>]><svg xmlns='http://www.w3.org/2000/svg' width='16' height='16'><text>&xxe;</text></svg>",
+        // Entity-expansion (billion laughs) denial of service
+        "<?xml version='1.0'?><!DOCTYPE svg [<!ENTITY a 'aaaaaaaaaa'><!ENTITY b '&a;&a;&a;&a;&a;&a;&a;&a;&a;&a;'><!ENTITY c '&b;&b;&b;&b;&b;&b;&b;&b;&b;&b;'><!ENTITY d '&c;&c;&c;&c;&c;&c;&c;&c;&c;&c;'><!ENTITY e '&d;&d;&d;&d;&d;&d;&d;&d;&d;&d;'><!ENTITY f '&e;&e;&e;&e;&e;&e;&e;&e;&e;&e;'>]><svg xmlns='http://www.w3.org/2000/svg' width='16' height='16'><text>&f;</text></svg>",
+        // Nested SVG in a base64 data: URI whose inner document references an external resource
+        "<svg xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink' width='16' height='16'><image xlink:href='data:image/svg+xml;base64,PHN2ZyB4bWxucz0naHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmcnIHhtbG5zOnhsaW5rPSdodHRwOi8vd3d3LnczLm9yZy8xOTk5L3hsaW5rJyB3aWR0aD0nOCcgaGVpZ2h0PSc4Jz48aW1hZ2UgeGxpbms6aHJlZj0naHR0cDovL2V4YW1wbGUuaW52YWxpZC9uZXN0ZWQucG5nJyB3aWR0aD0nOCcgaGVpZ2h0PSc4Jy8+PC9zdmc+' width='16' height='16'/></svg>",
+        // Nested SVG in a URL-encoded (non-base64) data: URI referencing an external resource
+        "<svg xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink' width='16' height='16'><image xlink:href='data:image/svg+xml,%3Csvg%20xmlns%3D%27http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%27%20xmlns%3Axlink%3D%27http%3A%2F%2Fwww.w3.org%2F1999%2Fxlink%27%3E%3Cimage%20xlink%3Ahref%3D%27file%3A%2F%2F%2Fetc%2Fpasswd%27%2F%3E%3C%2Fsvg%3E' width='16' height='16'/></svg>",
+    };
+
+    public static TheoryData<string> SafeSvgs => new()
+    {
+        "<svg xmlns='http://www.w3.org/2000/svg' width='16' height='16'><rect width='16' height='16' fill='red'/></svg>",
+        // Same-document fragment references are allowed
+        "<svg xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink' width='16' height='16'><defs><linearGradient id='g'/></defs><rect width='16' height='16' fill='url(#g)'/><use xlink:href='#g'/></svg>",
+        // Inline data URIs are allowed
+        "<svg xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink' width='16' height='16'><image xlink:href='data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==' width='16' height='16'/></svg>",
+        // A DOCTYPE without external entities is allowed
+        "<?xml version='1.0'?><!DOCTYPE svg PUBLIC '-//W3C//DTD SVG 1.1//EN' 'http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd'><svg xmlns='http://www.w3.org/2000/svg' width='16' height='16'><rect width='16' height='16'/></svg>",
+        // An internal general entity with no external reference is allowed (and is expanded by the renderer)
+        "<?xml version='1.0'?><!DOCTYPE svg [<!ENTITY col 'red'>]><svg xmlns='http://www.w3.org/2000/svg' width='16' height='16'><rect width='16' height='16' fill='&col;'/></svg>",
+        // A nested data:image/svg+xml payload that is itself self-contained is allowed
+        "<svg xmlns='http://www.w3.org/2000/svg' xmlns:xlink='http://www.w3.org/1999/xlink' width='16' height='16'><image xlink:href='data:image/svg+xml;base64,PHN2ZyB4bWxucz0naHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmcnIHdpZHRoPSc4JyBoZWlnaHQ9JzgnPjxyZWN0IHdpZHRoPSc4JyBoZWlnaHQ9JzgnIGZpbGw9J2JsdWUnLz48L3N2Zz4=' width='16' height='16'/></svg>",
+    };
+
+    [Theory]
+    [MemberData(nameof(ExternalReferenceSvgs))]
+    public static void IsSafe_ExternalReference_ReturnsFalse(string svg)
+    {
+        var path = WriteTemp(svg);
+        try
+        {
+            Assert.False(SvgSecurityValidator.IsSafe(path, NullLogger.Instance));
+        }
+        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, NullLogger.Instance));
+        }
+        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"), NullLogger.Instance));
+    }
+
+    private static string WriteTemp(string svg)
+    {
+        var path = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName() + ".svg");
+        File.WriteAllText(path, svg);
+        return path;
+    }
+}