diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/Jellyfin.Drawing.Skia/SkiaEncoder.cs | 8 | ||||
| -rw-r--r-- | src/Jellyfin.Drawing.Skia/SvgSecurityValidator.cs | 225 |
2 files changed, 134 insertions, 99 deletions
diff --git a/src/Jellyfin.Drawing.Skia/SkiaEncoder.cs b/src/Jellyfin.Drawing.Skia/SkiaEncoder.cs index 64329feabd..3e353db8de 100644 --- a/src/Jellyfin.Drawing.Skia/SkiaEncoder.cs +++ b/src/Jellyfin.Drawing.Skia/SkiaEncoder.cs @@ -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; } diff --git a/src/Jellyfin.Drawing.Skia/SvgSecurityValidator.cs b/src/Jellyfin.Drawing.Skia/SvgSecurityValidator.cs index 354c761eeb..f8a1d7d443 100644 --- a/src/Jellyfin.Drawing.Skia/SvgSecurityValidator.cs +++ b/src/Jellyfin.Drawing.Skia/SvgSecurityValidator.cs @@ -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)); + 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 (bytes.Length > 2 && bytes[0] == 0x1F && bytes[1] == 0x8B) + if (length > 2 && buffer[0] == 0x1F && buffer[1] == 0x8B) { - bytes = Decompress(bytes); + using var decompressed = Decompress(buffer, length); + return Validate(decompressed, depth + 1); } - using var ms = new MemoryStream(bytes, false); - return IsSafe(ms, logger, 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; + } + finally + { + if (buffer is not null) + { + ArrayPool<byte>.Shared.Return(buffer); + } } } - private static byte[] Decompress(byte[] compressed) + private static MemoryStream Decompress(byte[] compressed, int length) { - using var input = new MemoryStream(compressed, false); + using var input = new MemoryStream(compressed, 0, length, 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) + var output = new MemoryStream(); + var buffer = ArrayPool<byte>.Shared.Rent(DecompressBufferSize); + try { - total += read; - if (total > MaxDecompressedBytes) + var total = 0; + int read; + while ((read = gzip.Read(buffer, 0, buffer.Length)) > 0) { - throw new InvalidDataException("Compressed data URI exceeds the allowed size"); - } + total += read; + if (total > MaxDecompressedBytes) + { + throw new InvalidDataException("Compressed data URI exceeds the allowed size"); + } - output.Write(buffer, 0, read); + output.Write(buffer, 0, read); + } + } + catch + { + output.Dispose(); + throw; + } + finally + { + ArrayPool<byte>.Shared.Return(buffer); } - return output.ToArray(); + output.Position = 0; + return output; } - private static bool HasUnsafeCssReference(string? value, ILogger logger, int depth) + private static string? ValidateCss(ReadOnlySpan<char> value, int depth) { - if (string.IsNullOrEmpty(value)) + if (value.IsEmpty) { - return false; + 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; } } |