Skip to content

Harden Visio comments loading to prevent XML DoS #1927

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

Merged
PrzemyslawKlys merged 2 commits into from
Jun 16, 2026
Merged

Harden Visio comments loading to prevent XML DoS #1927

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
48e94a2
Harden Visio comments loading
PrzemyslawKlys Jun 16, 2026
7f4b605
Enforce Visio comment save limits
PrzemyslawKlys Jun 16, 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
113 changes: 113 additions & 0 deletions OfficeIMO.Tests/Visio.Comments.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.IO.Compression;
using System.Linq;
using System.Text;
using System.Xml.Linq;
using OfficeIMO.Visio;
using OfficeIMO.Visio.Fluent;
Expand Down Expand Up @@ -170,6 +172,74 @@ public void CommentsCanBeReviewedUpdatedReopenedAndRemovedInLoadedDocuments() {
AssertCommentXml(fluentPath, "Follow-up resolved", done: true, expectedEdited: finalResolvedAt);
}

[Fact]
public void LoadRejectsCommentsPartWithTooMuchXml() {
string filePath = CreateDocumentWithComment();
string oversizedCommentText = new('x', checked((int)VisioDocument.MaxCommentsXmlCharacters));
ReplaceCommentsXml(filePath, CreateCommentsXml(oversizedCommentText));

Assert.ThrowsAny<System.Xml.XmlException>(() => VisioDocument.Load(filePath));
}

[Fact]
public void LoadRejectsTooManyNativeComments() {
string filePath = CreateDocumentWithComment();
ReplaceCommentsXml(filePath, CreateCommentsXml(Enumerable.Range(0, VisioDocument.MaxLoadedComments + 1)
.Select(index => "Comment " + index.ToString())));

InvalidDataException exception = Assert.Throws<InvalidDataException>(() => VisioDocument.Load(filePath));
Assert.Contains(VisioDocument.MaxLoadedComments.ToString(), exception.Message);
}

[Fact]
public void LoadRejectsOversizedNativeCommentText() {
string filePath = CreateDocumentWithComment();
string oversizedCommentText = new('x', VisioDocument.MaxCommentTextCharacters + 1);
ReplaceCommentsXml(filePath, CreateCommentsXml(oversizedCommentText));

InvalidDataException exception = Assert.Throws<InvalidDataException>(() => VisioDocument.Load(filePath));
Assert.Contains(VisioDocument.MaxCommentTextCharacters.ToString(), exception.Message);
}

[Fact]
public void SaveRejectsTooManyNativeComments() {
string filePath = Path.Combine(Path.GetTempPath(), Guid.NewGuid() + ".vsdx");
VisioDocument document = VisioDocument.Create(filePath);
VisioPage page = document.AddPage("Review", 11, 8.5);
for (int index = 0; index <= VisioDocument.MaxLoadedComments; index++) {
page.Comments.Add(new VisioComment("Comment " + index.ToString()) {
AuthorName = "Operations",
AuthorInitials = "OP"
});
}

InvalidDataException exception = Assert.Throws<InvalidDataException>(() => document.Save());
Assert.Contains(VisioDocument.MaxLoadedComments.ToString(), exception.Message);
}

[Fact]
public void SaveRejectsOversizedNativeCommentText() {
string filePath = Path.Combine(Path.GetTempPath(), Guid.NewGuid() + ".vsdx");
VisioDocument document = VisioDocument.Create(filePath);
VisioPage page = document.AddPage("Review", 11, 8.5);
page.AddComment(new string('x', VisioDocument.MaxCommentTextCharacters + 1), "Operations", "OP");

InvalidDataException exception = Assert.Throws<InvalidDataException>(() => document.Save());
Assert.Contains(VisioDocument.MaxCommentTextCharacters.ToString(), exception.Message);
}

[Fact]
public void SaveRejectsCommentsPartThatExceedsUtf8ByteLimit() {
string filePath = Path.Combine(Path.GetTempPath(), Guid.NewGuid() + ".vsdx");
VisioDocument document = VisioDocument.Create(filePath);
VisioPage page = document.AddPage("Review", 11, 8.5);
VisioComment comment = page.AddComment("Byte budget", "Operations", "OP");
comment.AuthorName = new string('\u20ac', checked((int)(VisioDocument.MaxCommentsPartBytes / 3L + 1024L)));

InvalidDataException exception = Assert.Throws<InvalidDataException>(() => document.Save());
Assert.Contains(VisioDocument.MaxCommentsPartBytes.ToString(), exception.Message);
}

private static void AssertNativeCommentPackage(
string filePath,
string expectedText,
Expand Down Expand Up @@ -244,6 +314,49 @@ private static void AssertCommentTextAbsent(string filePath, string text) {
Assert.DoesNotContain(comments.Descendants(v + "CommentEntry"), entry => entry.Value == text);
}

private static string CreateDocumentWithComment() {
string filePath = Path.Combine(Path.GetTempPath(), Guid.NewGuid() + ".vsdx");
VisioDocument document = VisioDocument.Create(filePath);
VisioPage page = document.AddPage("Review", 11, 8.5);
page.AddComment("Initial comment", "Operations", "OP");
document.Save();
return filePath;
}

private static void ReplaceCommentsXml(string filePath, string commentsXml) {
using ZipArchive archive = ZipFile.Open(filePath, ZipArchiveMode.Update);
ZipArchiveEntry entry = archive.GetEntry("visio/comments.xml") ?? throw new InvalidOperationException("Missing comments part.");
entry.Delete();
ZipArchiveEntry replacement = archive.CreateEntry("visio/comments.xml", CompressionLevel.Optimal);
using Stream stream = replacement.Open();
using StreamWriter writer = new(stream, new UTF8Encoding(false));
writer.Write(commentsXml);
}

private static string CreateCommentsXml(string commentText) {
return CreateCommentsXml(new[] { commentText });
}

private static string CreateCommentsXml(IEnumerable<string> commentTexts) {
XNamespace v = "http://schemas.microsoft.com/office/visio/2012/main";
int id = 1;
XDocument comments = new(new XElement(v + "Comments",
new XAttribute("xmlns", v.NamespaceName),
new XElement(v + "AuthorList",
new XElement(v + "AuthorEntry",
new XAttribute("ID", "1"),
new XAttribute("Name", "Operations"),
new XAttribute("Initials", "OP"))),
new XElement(v + "CommentList",
commentTexts.Select(text => new XElement(v + "CommentEntry",
new XAttribute("IX", id++),
new XAttribute("AuthorID", "1"),
new XAttribute("PageID", "0"),
text)))));

return comments.ToString(SaveOptions.DisableFormatting);
}

private static string FormatExpectedDate(DateTimeOffset value) {
return System.Xml.XmlConvert.ToString(value.UtcDateTime, System.Xml.XmlDateTimeSerializationMode.Utc);
}
Expand Down
93 changes: 91 additions & 2 deletions OfficeIMO.Visio/VisioDocument.LoadCore.Comments.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,26 @@
using System;
using System.Collections.Generic;
using System.Globalization;
using System.IO;
using System.IO.Packaging;
using System.Linq;
using System.Xml;
using System.Xml.Linq;

namespace OfficeIMO.Visio {
public partial class VisioDocument {
internal const long MaxCommentsPartBytes = 12_000_000;
internal const long MaxCommentsXmlCharacters = 10_000_000;
internal const int MaxLoadedComments = 10_000;
internal const int MaxCommentTextCharacters = 32_768;

private static readonly XmlReaderSettings CommentsXmlReaderSettings = new() {
DtdProcessing = DtdProcessing.Prohibit,
XmlResolver = null,
MaxCharactersInDocument = MaxCommentsXmlCharacters,
MaxCharactersFromEntities = 0,
};

private static void LoadComments(Package package, PackagePart documentPart, VisioDocument document) {
PackageRelationship? commentsRel = documentPart.GetRelationshipsByType(CommentsRelationshipType).FirstOrDefault();
if (commentsRel == null) {
Expand All @@ -20,7 +33,7 @@ private static void LoadComments(Package package, PackagePart documentPart, Visi
}

PackagePart commentsPart = package.GetPart(commentsUri);
XDocument commentsXml = XDocument.Load(commentsPart.GetStream());
XDocument commentsXml = LoadCommentsXml(commentsPart);
XElement? root = commentsXml.Root;
if (root == null) {
return;
Expand All @@ -40,7 +53,14 @@ private static void LoadComments(Package package, PackagePart documentPart, Visi
}

XElement? commentList = root.Elements().FirstOrDefault(element => IsVisioElement(element, "CommentList"));
int loadedCommentCount = 0;
foreach (XElement commentElement in commentList?.Elements().Where(element => IsVisioElement(element, "CommentEntry")) ?? Enumerable.Empty<XElement>()) {
loadedCommentCount++;
if (loadedCommentCount > MaxLoadedComments) {
throw new InvalidDataException($"Visio comments part contains more than {MaxLoadedComments} comments.");
Comment thread
PrzemyslawKlys marked this conversation as resolved.
}

string commentText = GetBoundedCommentText(commentElement);
if (!TryParseIntAttribute(commentElement, "PageID", out int pageId)) {
continue;
}
Expand All @@ -53,7 +73,7 @@ private static void LoadComments(Package package, PackagePart documentPart, Visi
TryParseIntAttribute(commentElement, "IX", out int commentId);
TryParseIntAttribute(commentElement, "AuthorID", out int authorId);
authors.TryGetValue(authorId, out var author);
VisioComment comment = new(commentElement.Value) {
VisioComment comment = new(commentText) {
Id = commentId > 0 ? commentId : GetNextLoadedCommentId(page),
AuthorName = author.Name,
AuthorInitials = author.Initials,
Expand All @@ -73,6 +93,25 @@ private static void LoadComments(Package package, PackagePart documentPart, Visi
}
}

private static XDocument LoadCommentsXml(PackagePart commentsPart) {
using Stream commentsStream = commentsPart.GetStream();
using Stream boundedStream = new BoundedReadStream(commentsStream, MaxCommentsPartBytes);
using XmlReader reader = XmlReader.Create(boundedStream, CommentsXmlReaderSettings);
Comment thread
PrzemyslawKlys marked this conversation as resolved.
return XDocument.Load(reader);
}

private static string GetBoundedCommentText(XElement commentElement) {
int textLength = 0;
foreach (XText text in commentElement.DescendantNodes().OfType<XText>()) {
textLength += text.Value.Length;
if (textLength > MaxCommentTextCharacters) {
throw new InvalidDataException($"Visio comment text exceeds {MaxCommentTextCharacters} characters.");
Comment thread
PrzemyslawKlys marked this conversation as resolved.
}
}

return commentElement.Value;
}

private static bool IsVisioElement(XElement element, string localName) {
return string.Equals(element.Name.LocalName, localName, StringComparison.OrdinalIgnoreCase);
}
Expand Down Expand Up @@ -134,5 +173,55 @@ private static bool ParseCommentBool(string? value) {

return persistedId;
}

private sealed class BoundedReadStream : Stream {
private readonly Stream _inner;
private readonly long _maxBytes;
private long _bytesRead;

internal BoundedReadStream(Stream inner, long maxBytes) {
_inner = inner ?? throw new ArgumentNullException(nameof(inner));
_maxBytes = maxBytes;
}

public override bool CanRead => _inner.CanRead;

public override bool CanSeek => false;

public override bool CanWrite => false;

public override long Length => throw new NotSupportedException();

public override long Position {
get => _bytesRead;
set => throw new NotSupportedException();
}

public override void Flush() {
_inner.Flush();
}

public override int Read(byte[] buffer, int offset, int count) {
int read = _inner.Read(buffer, offset, count);
_bytesRead += read;
if (_bytesRead > _maxBytes) {
throw new InvalidDataException($"Visio comments part exceeds {_maxBytes} bytes.");
}

return read;
}

public override long Seek(long offset, SeekOrigin origin) {
throw new NotSupportedException();
}

public override void SetLength(long value) {
throw new NotSupportedException();
}

public override void Write(byte[] buffer, int offset, int count) {
throw new NotSupportedException();
}
}
}
}
27 changes: 26 additions & 1 deletion OfficeIMO.Visio/VisioDocument.SaveCore.Comments.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,17 @@ private static void WriteCommentsPart(

foreach (VisioPage page in pages) {
foreach (VisioComment comment in page.Comments) {
ValidateCommentForSave(comment);
CommentAuthorKey authorKey = new(comment.AuthorName, comment.AuthorInitials, comment.AuthorResolutionId);
if (!authorIds.TryGetValue(authorKey, out int authorId)) {
authorId = authorIds.Count + 1;
authorIds.Add(authorKey, authorId);
}

comments.Add((page, comment, authorId));
if (comments.Count > MaxLoadedComments) {
throw new InvalidDataException($"Visio comments part contains more than {MaxLoadedComments} comments.");
}
}
}

Expand Down Expand Up @@ -104,9 +108,30 @@ private static void WriteCommentsPart(
authorList,
commentList));

string serializedComments = commentsXml.Declaration + Environment.NewLine + commentsXml.ToString(SaveOptions.DisableFormatting);
ValidateCommentsXmlForSave(serializedComments);

using Stream stream = commentsPart.GetStream(FileMode.Create, FileAccess.Write);
using StreamWriter writer = new(stream, new UTF8Encoding(false));
writer.Write(commentsXml.Declaration + Environment.NewLine + commentsXml.ToString(SaveOptions.DisableFormatting));
writer.Write(serializedComments);
}

private static void ValidateCommentForSave(VisioComment comment) {
string text = comment.Text ?? string.Empty;
if (text.Length > MaxCommentTextCharacters) {
throw new InvalidDataException($"Visio comment text exceeds {MaxCommentTextCharacters} characters.");
}
}

private static void ValidateCommentsXmlForSave(string commentsXml) {
if (commentsXml.Length > MaxCommentsXmlCharacters) {
throw new InvalidDataException($"Visio comments part exceeds {MaxCommentsXmlCharacters} XML characters.");
}

int byteCount = Encoding.UTF8.GetByteCount(commentsXml);
if (byteCount > MaxCommentsPartBytes) {
throw new InvalidDataException($"Visio comments part exceeds {MaxCommentsPartBytes} bytes.");
}
}

private static int AssignSaveFallbackCommentId(VisioComment comment, XElement commentList) {
Expand Down
Loading