unkin/jellyfin-ha-src
4
0
Fork 0
Code Issues 10 Pull Requests 2 Packages Projects Releases Wiki Activity

Apply review suggestions

Browse Source
This commit is contained in:
Shadowghost
2026-08-28 07:21:37 +02:00
parent cefa78fc1d
commit 99f21f1662
3 changed files with 150 additions and 109 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Jellyfin.Drawing.Skia/SkiaEncoder.cs
+4 -4
View File
@@ -191,9 +191,9 @@ public class SkiaEncoder : IImageEncoder
var extension = Path.GetExtension(path.AsSpan());
if (extension.Equals(".svg", StringComparison.OrdinalIgnoreCase))
{
if (!SvgSecurityValidator.IsSafe(path, _logger))
if (!SvgSecurityValidator.IsSafe(path, out var reason))
{
_logger.LogError("Refusing to determine dimensions for SVG with external references {FilePath}", path);
_logger.LogError("Refusing to determine dimensions for SVG {FilePath}: {Reason}", path, reason);
return default;
}
@@ -459,9 +459,9 @@ public class SkiaEncoder : IImageEncoder
throw new FileNotFoundException("File not found", path);
}
if (!SvgSecurityValidator.IsSafe(path, _logger))
if (!SvgSecurityValidator.IsSafe(path, out var reason))
{
_logger.LogError("Refusing to render SVG with external references {FilePath}", path);
_logger.LogError("Refusing to render SVG {FilePath}: {Reason}", path, reason);
return null;
}
src/Jellyfin.Drawing.Skia/SvgSecurityValidator.cs
+136 -101
View File
@@ -1,10 +1,11 @@
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;
using Microsoft.Extensions.Logging;
[assembly: InternalsVisibleTo("Jellyfin.Drawing.Skia.Tests")]
@@ -21,6 +22,8 @@ internal static class SvgSecurityValidator
// 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,
@@ -37,37 +40,40 @@ internal static class SvgSecurityValidator
/// to external resources.
/// </summary>
/// <param name="path">The path to the SVG file.</param>
/// <param name="logger">The logger.</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, ILogger logger)
public static bool IsSafe(string path, [NotNullWhen(false)] out string? reason)
{
try
{
using var stream = File.OpenRead(path);
return IsSafe(stream, logger);
reason = Validate(stream, 0);
}
catch (IOException ex)
{
logger.LogWarning(ex, "Unable to read SVG {Path} for validation, refusing to render", path);
return false;
reason = "Unable to read the file for validation: " + ex.Message;
}
catch (UnauthorizedAccessException ex)
{
logger.LogWarning(ex, "Unable to read SVG {Path} for validation, refusing to render", path);
return false;
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="logger">The logger.</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, ILogger logger)
=> IsSafe(stream, logger, 0);
public static bool IsSafe(Stream stream, [NotNullWhen(false)] out string? reason)
{
reason = Validate(stream, 0);
return reason is null;
}
private static bool IsSafe(Stream stream, ILogger logger, int depth)
private static string? Validate(Stream stream, int depth)
{
try
{
@@ -77,194 +83,219 @@ internal static class SvgSecurityValidator
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;
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 value = reader.Value;
if (reader.LocalName.Equals("href", StringComparison.OrdinalIgnoreCase))
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)
{
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;
return reason;
}
}
reader.MoveToElement();
break;
}
case XmlNodeType.Text:
case XmlNodeType.CDATA:
if (HasUnsafeCssReference(reader.Value, logger, depth))
{
var reason = ValidateCss(reader.Value, depth);
if (reason is not null)
{
logger.LogWarning("Refusing to render SVG referencing external resource in style block");
return false;
return reason;
}
break;
}
}
}
return true;
return null;
}
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;
return "The document could not be safely parsed: " + ex.Message;
}
}
private static bool IsReferenceSafe(string? value, ILogger logger, int depth)
private static string? ValidateReference(ReadOnlySpan<char> value, int depth, string context)
{
if (string.IsNullOrEmpty(value))
{
return true;
}
var trimmed = value.Trim();
if (trimmed.Length == 0 || trimmed[0] == '#')
if (trimmed.IsEmpty || trimmed[0] == '#')
{
return true;
return null;
}
if (trimmed.StartsWith("data:", StringComparison.OrdinalIgnoreCase))
{
return IsDataUriSafe(trimmed, logger, depth);
return ValidateDataUri(trimmed, depth, context);
}
return false;
return "An external resource is referenced via " + context;
}
private static bool IsDataUriSafe(string dataUri, ILogger logger, int depth)
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(',', StringComparison.Ordinal);
var comma = dataUri.IndexOf(',');
if (comma < 0)
{
return false;
return "A malformed data URI is referenced via " + context;
}
var header = dataUri[5..comma];
var segments = header.Split(';');
var mediaType = segments.Length > 0 && segments[0].Contains('/', StringComparison.Ordinal)
? segments[0].Trim()
: "text/plain";
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.Equals("image/svg+xml", StringComparison.OrdinalIgnoreCase))
if (!mediaType.Contains('/') || !mediaType.Equals("image/svg+xml", StringComparison.OrdinalIgnoreCase))
{
return true;
return null;
}
if (depth >= MaxDataUriDepth)
{
logger.LogWarning("Refusing to render SVG with nested data URIs exceeding the allowed depth");
return false;
return "Nested data URIs exceed the allowed depth";
}
var isBase64 = segments.Length > 0 && segments[^1].Trim().Equals("base64", StringComparison.OrdinalIgnoreCase);
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
{
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)
int length;
if (isBase64)
{
bytes = Decompress(bytes);
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);
}
using var ms = new MemoryStream(bytes, false);
return IsSafe(ms, logger, depth + 1);
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)
{
logger.LogWarning(ex, "Refusing to render SVG with an undecodable data URI");
return false;
return "An undecodable data URI is referenced via " + context + ": " + ex.Message;
}
catch (InvalidDataException ex)
{
logger.LogWarning(ex, "Refusing to render SVG with an invalid compressed data URI");
return false;
return "An invalid compressed data URI is referenced via " + context + ": " + ex.Message;
}
}
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)
finally
{
total += read;
if (total > MaxDecompressedBytes)
if (buffer is not null)
{
throw new InvalidDataException("Compressed data URI exceeds the allowed size");
ArrayPool<byte>.Shared.Return(buffer);
}
output.Write(buffer, 0, read);
}
return output.ToArray();
}
private static bool HasUnsafeCssReference(string? value, ILogger logger, int depth)
private static MemoryStream Decompress(byte[] compressed, int length)
{
if (string.IsNullOrEmpty(value))
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
{
return false;
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 span = value.AsSpan();
var index = 0;
while (true)
{
var found = span[index..].IndexOf("url(", StringComparison.OrdinalIgnoreCase);
var found = value[index..].IndexOf("url(", StringComparison.OrdinalIgnoreCase);
if (found < 0)
{
break;
}
var start = index + found + 4;
var close = span[start..].IndexOf(')');
var close = value[start..].IndexOf(')');
if (close < 0)
{
break;
}
var target = span.Slice(start, close).Trim();
var target = value.Slice(start, close).Trim();
target = target.Trim('\'');
target = target.Trim('"').Trim();
if (!IsReferenceSafe(target.ToString(), logger, depth))
var reason = ValidateReference(target, depth, "url()");
if (reason is not null)
{
return true;
return reason;
}
index = start + close + 1;
if (index >= span.Length)
if (index >= value.Length)
{
break;
}
@@ -274,31 +305,35 @@ internal static class SvgSecurityValidator
index = 0;
while (true)
{
var found = span[index..].IndexOf("@import", StringComparison.OrdinalIgnoreCase);
var found = value[index..].IndexOf("@import", StringComparison.OrdinalIgnoreCase);
if (found < 0)
{
break;
}
var rest = span[(index + found + 7)..];
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 && !IsReferenceSafe(afterQuote[..end].Trim().ToString(), logger, depth))
if (end >= 0)
{
return true;
var reason = ValidateReference(afterQuote[..end], depth, "@import");
if (reason is not null)
{
return reason;
}
}
}
index = index + found + 7;
if (index >= span.Length)
if (index >= value.Length)
{
break;
}
}
return false;
return null;
}
}
tests/Jellyfin.Drawing.Skia.Tests/SvgSecurityValidatorTests.cs
+10 -4
View File
@@ -1,5 +1,4 @@
using System.IO;
using Microsoft.Extensions.Logging.Abstractions;
using Xunit;
namespace Jellyfin.Drawing.Skia.Tests;
@@ -31,6 +30,8 @@ public static class SvgSecurityValidatorTests
"<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()
@@ -46,6 +47,8 @@ public static class SvgSecurityValidatorTests
"<?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]
@@ -55,7 +58,8 @@ public static class SvgSecurityValidatorTests
var path = WriteTemp(svg);
try
{
Assert.False(SvgSecurityValidator.IsSafe(path, NullLogger.Instance));
Assert.False(SvgSecurityValidator.IsSafe(path, out var reason));
Assert.NotNull(reason);
}
finally
{
@@ -70,7 +74,8 @@ public static class SvgSecurityValidatorTests
var path = WriteTemp(svg);
try
{
Assert.True(SvgSecurityValidator.IsSafe(path, NullLogger.Instance));
Assert.True(SvgSecurityValidator.IsSafe(path, out var reason));
Assert.Null(reason);
}
finally
{
@@ -81,7 +86,8 @@ public static class SvgSecurityValidatorTests
[Fact]
public static void IsSafe_MissingFile_ReturnsFalse()
{
Assert.False(SvgSecurityValidator.IsSafe(Path.Combine(Path.GetTempPath(), "does-not-exist-" + Path.GetRandomFileName() + ".svg"), NullLogger.Instance));
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)