Skip to content

Fix stored XSS via malicious SVG upload in File Management #2407

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Copilot wants to merge 2 commits into
base: dev
Choose a base branch
from
Draft

Fix stored XSS via malicious SVG upload in File Management #2407

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0ba0779
Initial plan
Copilot Mar 30, 2026
81dad48
Fix stored XSS via SVG upload: sanitize SVG content before storage
Copilot Mar 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 94 additions & 0 deletions src/Backend/FluentCMS.Services/FileService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
using FluentCMS.Providers.FileStorageProviders;
using System.Text.RegularExpressions;
using System.Xml;
using System.Xml.Linq;

namespace FluentCMS.Services;

Expand Down Expand Up @@ -28,6 +30,13 @@ public async Task<File> Create(File file, System.IO.Stream fileContent, Cancella

file.NormalizedName = GetNormalizedFileName(file.Name);

// Sanitize SVG content before persisting to remove potential XSS payloads
if (IsSvgFile(file))
{
fileContent = SanitizeSvg(fileContent);
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;

Copilot uses AI. Check for mistakes.
file.Size = fileContent.Length;
}

// check if file with the same name already exists
var existingFile = await fileRepository.GetByName(folder.SiteId, folder.Id, file.NormalizedName, cancellationToken);
if (existingFile != null)
Expand Down Expand Up @@ -155,4 +164,89 @@ private static string GetNormalizedFileName(string fileName)
var normalized = fileName.Trim().ToLower();
return normalized;
}

private static bool IsSvgFile(File file)
{
return string.Equals(file.Extension, ".svg", StringComparison.OrdinalIgnoreCase) ||
string.Equals(file.ContentType, "image/svg+xml", StringComparison.OrdinalIgnoreCase);
}

// Dangerous SVG element local names
private static readonly HashSet<string> _dangerousElements = new(StringComparer.OrdinalIgnoreCase)
{
"script",
"foreignObject",
};

// URL-bearing attribute local names whose values must not use dangerous schemes
private static readonly HashSet<string> _urlAttributeLocalNames = new(StringComparer.OrdinalIgnoreCase)
{
"href",
"action",
"src",
};

// Dangerous URI schemes (allowlist approach would be better but this covers the known vectors)
private static readonly string[] _dangerousSchemes = ["javascript:", "vbscript:", "data:"];

private static readonly XNamespace _xlinkNs = "http://www.w3.org/1999/xlink";

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);
Comment on lines +194 to +212
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
}

// Remove dangerous elements (e.g. <script>, <foreignObject>)
var elementsToRemove = doc.Descendants()
.Where(e => _dangerousElements.Contains(e.Name.LocalName))
.ToList();

foreach (var element in elementsToRemove)
element.Remove();

// Remove event-handler attributes (on*) and URL attributes with dangerous schemes
var attributesToRemove = doc.Descendants()
.SelectMany(e => e.Attributes())
.Where(a =>
a.Name.LocalName.StartsWith("on", StringComparison.OrdinalIgnoreCase) ||
IsUrlAttributeWithDangerousScheme(a))
.ToList();

foreach (var attribute in attributesToRemove)
attribute.Remove();

var ms = new System.IO.MemoryStream();
doc.Save(ms);
ms.Position = 0;
return ms;
}

private static bool IsUrlAttributeWithDangerousScheme(XAttribute attribute)
{
// Check both local href and xlink:href
bool isUrlAttr = _urlAttributeLocalNames.Contains(attribute.Name.LocalName) ||
attribute.Name == _xlinkNs + "href";

if (!isUrlAttr)
return false;

var value = attribute.Value.TrimStart();
return _dangerousSchemes.Any(scheme => value.StartsWith(scheme, StringComparison.OrdinalIgnoreCase));
}
}
Loading