Fix stored XSS via malicious SVG upload in File Management

[Copilot]

Authenticated admins could upload SVG files containing embedded JavaScript (<script>, event handler attributes, javascript: URIs) that executed in any visitor's browser when the file URL was accessed directly.

Changes

FileService.cs — server-side SVG sanitization before storage

• Sanitization runs in Create() before fileRepository.Create() so stored file size stays consistent with sanitized content
• Strips dangerous elements: <script>, <foreignObject>
• Strips all on* event-handler attributes (onload, onclick, onerror, etc.)
• Strips href, xlink:href (matched via proper XML namespace http://www.w3.org/1999/xlink), src, and action attributes whose values begin with javascript:, vbscript:, or data: schemes
• XXE prevented via DtdProcessing.Prohibit / XmlResolver = null
• Non-XML-parseable SVGs fall back to an empty safe <svg> (catches XmlException only)
• No new dependencies — uses System.Xml.Linq from BCL

// Sanitize SVG content before persisting to remove potential XSS payloads
if (IsSvgFile(file))
{
    fileContent = SanitizeSvg(fileContent);
    file.Size = fileContent.Length;
}

[Copilot]

Pull request overview

This PR addresses a stored XSS vulnerability in the asset/file management flow by sanitizing uploaded SVGs server-side before persisting and serving them.

Changes:

• Adds SVG detection and sanitization in FileService.Create() prior to repository/storage writes.
• Implements XML-based SVG sanitization that removes dangerous elements (script, foreignObject) and strips event-handler (on*) and dangerous URL attributes/schemes.
• Hardens XML parsing against XXE by prohibiting DTD processing and disabling the XML resolver, with a fallback to an empty safe SVG on XML parse errors.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

SanitizeSvg parses untrusted input with XDocument.Load(reader) and no explicit document size limits, and it also ignores the request CancellationToken. A large SVG upload can cause high CPU/memory usage (potential DoS) and won’t be interruptible on client disconnect. Consider enforcing a maximum SVG size before parsing and configuring XmlReaderSettings.MaxCharactersInDocument (and/or similar guards), and/or switching to async loading that can be cancelled.

Suggested change

	private static System.IO.MemoryStream SanitizeSvg(System.IO.Stream svgStream)
	{
	XDocument doc;
	try
	{
	// Disable DTD processing to prevent XXE attacks
	var readerSettings = new XmlReaderSettings
	{
	DtdProcessing = DtdProcessing.Prohibit,
	XmlResolver = null,
	};
	using var reader = XmlReader.Create(svgStream, readerSettings);
	doc = XDocument.Load(reader);
	}
	catch (XmlException)
	{
	// If the SVG cannot be parsed as XML return an empty SVG
	var empty = System.Text.Encoding.UTF8.GetBytes("<svg xmlns=\"http://www.w3.org/2000/svg\"></svg>");
	return new System.IO.MemoryStream(empty);
	// Maximum allowed SVG size in bytes to prevent excessive memory/CPU usage when parsing.
	// This is a defense-in-depth limit; valid SVGs should normally be much smaller.
	private static readonly long _maxSvgBytes = 5 * 1024 * 1024; // 5 MB
	private static System.IO.MemoryStream SanitizeSvg(System.IO.Stream svgStream)
	{
	// Helper: return a minimal safe SVG document
	static System.IO.MemoryStream EmptySvg()
	{
	var empty = System.Text.Encoding.UTF8.GetBytes("<svg xmlns=\"http://www.w3.org/2000/svg\"></svg>");
	return new System.IO.MemoryStream(empty);
	}
	// Normalize input into a bounded MemoryStream so we can enforce size limits even for
	// non-seekable streams.
	System.IO.Stream effectiveStream = svgStream;
	if (svgStream is null)
	{
	return EmptySvg();
	}
	if (svgStream.CanSeek)
	{
	// Ensure we read from the beginning if possible
	if (svgStream.Position != 0)
	svgStream.Position = 0;
	if (svgStream.Length > _maxSvgBytes)
	{
	// Too large to process safely
	return EmptySvg();
	}
	}
	else
	{
	// Copy into a bounded MemoryStream and enforce the limit while reading
	var bounded = new System.IO.MemoryStream();
	var buffer = new byte[81920];
	long totalRead = 0;
	int read;
	while ((read = svgStream.Read(buffer, 0, buffer.Length)) > 0)
	{
	totalRead += read;
	if (totalRead > _maxSvgBytes)
	{
	return EmptySvg();
	}
	bounded.Write(buffer, 0, read);
	}
	bounded.Position = 0;
	effectiveStream = bounded;
	}
	XDocument doc;
	try
	{
	// Disable DTD processing to prevent XXE attacks and set explicit size limits
	var readerSettings = new XmlReaderSettings
	{
	DtdProcessing = DtdProcessing.Prohibit,
	XmlResolver = null,
	// Limit the total number of characters to mitigate DoS via oversized documents
	MaxCharactersInDocument = 10 * 1024 * 1024 // 10 MB of text
	};
	if (effectiveStream.CanSeek && effectiveStream.Position != 0)
	effectiveStream.Position = 0;
	using var reader = XmlReader.Create(effectiveStream, readerSettings);
	doc = XDocument.Load(reader);
	}
	catch (XmlException)
	{
	// If the SVG cannot be parsed as XML return an empty SVG
	return EmptySvg();

[Copilot]

SanitizeSvg returns a new MemoryStream, but Create never disposes it after fileStorageProvider.Upload(...) completes. For large SVGs this can keep sizable buffers alive until GC and increase memory pressure. Consider scoping the sanitized stream in a using and disposing it after upload (while still leaving non-sanitized streams under the caller’s ownership).

Suggested change

	fileContent = SanitizeSvg(fileContent);
	using var sanitizedStream = SanitizeSvg(fileContent);
	fileContent = sanitizedStream;