Merge commit from fork

Prevent SSRF, local file disclosure and DoS via external references in SVG rendering
This commit is contained in:
Cody Robibero
2026-08-30 14:40:03 -04:00
committed by GitHub
5 changed files with 491 additions and 0 deletions
+7
View File
@@ -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}
+20
View File
@@ -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, 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)
{
@@ -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;
/// <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 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
};
/// <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="reason">When this method returns <c>false</c>, the reason the document was rejected.</param>
/// <returns><c>true</c> if the document is free of external references; otherwise <c>false</c>.</returns>
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;
}
/// <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="reason">When this method returns <c>false</c>, the reason the document was rejected.</param>
/// <returns><c>true</c> if the document is free of external references; otherwise <c>false</c>.</returns>
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<char> 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<char> dataUri, int depth, string context)
{
// "data:[<mediatype>][;base64],<payload>" (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<byte>.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<byte>.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<byte>.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<byte>.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<byte>.Shared.Return(buffer);
}
output.Position = 0;
return output;
}
private static string? ValidateCss(ReadOnlySpan<char> 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;
}
}
@@ -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,99 @@
using System.IO;
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>",
// Nested gzip-compressed (svgz) 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,H4sIAAAAAAAC/23OwQrDIBAE0F/x5s217aWK8V+E2N2laiWRKP36Nin0lNvAPIZx64Zi5FTWSVJr1QL03lW/qdeCcNVaw1fIH7EjcXmewYsxBo5Wis5zo0nepaDISG2P3nEOGMVBLC3x8V+JI+SaouKyhcQz4FvVgucz4N1+x38AdK4P3LYAAAA=' 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>",
// A self-contained gzip-compressed (svgz) data: URI 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,H4sIAAAAAAAC/22Muw6AIAwAf6VbN0p0MQb4GBWBBB+Bav18ZXe75C5n6h3g2fJeLUbmcyQSESW9OkqgTmtNX4EgaeFocUCIPoXIDZ0pfuZfBWvK2eKUL4/kTHu4F2NB6oFrAAAA' 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, 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;
}
}