diff --git a/docs/security/threat-model.md b/docs/security/threat-model.md index 37506ff..d4b8305 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -46,7 +46,7 @@ physical directory. It does not authorize native commands or remote resource acc | Image traversal, device/ADS paths or encoded separators | Core path-policy tests pass; denied before filesystem access | | Junction/symlink escape or check/reopen race | Check normalized physical path on opened image handle; read same handle; real junction rejection test passes | | Remote image tracking / resource exfiltration | No-referrer header, no document networking; actual listening-server browser denial passed on 77dd04d | -| Oversized input / image denial of service | Pre-parse character bound and image byte bound; native limit tests pass; performance acceptance remains | +| Oversized input / image denial of service | Pre-parse character bound and image byte bound; native limit tests pass. Element nesting is bounded at 256 levels before any library walks the parsed tree recursively, with an early count that stops runaway nesting while it is still being parsed; deeper markup is refused as an unsupported document. Parse time is not bounded: it grows with document size, runs off the window thread, and can be cancelled between the render, the parse and each heading | | Settings corruption / UI exception | AppKit shell sanitation and crash handling; inherited foundation tests | | Tampered installer/update | Fail-closed signing/provenance/checksum release pipeline; real Azure CI authorization and signed artifacts remain unverified | | Privileged repository review abuse | Trusted review workflow only; current-SHA human acknowledgement remains required | diff --git a/src/MarkdownViewr.Windows.Core/Documents/DocumentOutline.cs b/src/MarkdownViewr.Windows.Core/Documents/DocumentOutline.cs index 2864589..935a34f 100644 --- a/src/MarkdownViewr.Windows.Core/Documents/DocumentOutline.cs +++ b/src/MarkdownViewr.Windows.Core/Documents/DocumentOutline.cs @@ -18,16 +18,25 @@ public sealed class DocumentOutline - /// Sanitizes first; authored identifiers cannot impersonate the generated navigation targets. - public DocumentOutline(string markdown, FrontmatterMode frontmatter = FrontmatterMode.Hide, MarkdownExtensions? extensions = null) + /// + /// Sanitizes first; authored identifiers cannot impersonate the generated navigation targets. + /// A large document takes time that grows with its size: cancellation is honoured between the + /// render, the parse and each heading, so a closed window or a newer load does not wait for it. + /// + public DocumentOutline(string markdown, FrontmatterMode frontmatter = FrontmatterMode.Hide, MarkdownExtensions? extensions = null, CancellationToken cancellation = default) { var sources = new DocumentSourceMap(markdown); var sourceHeadings = new List(); - using AngleSharp.Html.Dom.IHtmlDocument document = new HtmlParser().ParseDocument(new MarkdownRenderer(extensions).RenderWithSources(markdown, frontmatter, sources)); + cancellation.ThrowIfCancellationRequested(); + string rendered = new MarkdownRenderer(extensions).RenderWithSources(markdown, frontmatter, sources); + cancellation.ThrowIfCancellationRequested(); + using AngleSharp.Html.Dom.IHtmlDocument document = new HtmlParser().ParseDocument(rendered); IElement body = document.Body!; var fragments = new Dictionary(StringComparer.Ordinal); + var nextSuffix = new Dictionary(StringComparer.Ordinal); foreach(IElement element in body.QuerySelectorAll("h1,h2,h3,h4,h5,h6")) { + cancellation.ThrowIfCancellationRequested(); var heading = new DocumentHeading(_headings.Count, element.LocalName[1] - '0', element.TextContent.Trim()); _headings.Add(heading); if(element.GetAttribute(sources.AttributeName) is { } identity && sources.TryGetPosition(identity, out (int Start, int End) position)) @@ -37,11 +46,15 @@ public sealed class DocumentOutline _elements.Add(element, heading); element.Id = "mv-heading-" + heading.Index.ToString(CultureInfo.InvariantCulture); string slug = Markdig.Helpers.LinkHelper.UrilizeAsGfm(heading.Title); + // Repeated titles continue from the last suffix used for that title instead of rescanning from one. string fragment = slug; - for(int suffix = 1; fragments.ContainsKey(fragment); suffix++) + int suffix = nextSuffix.GetValueOrDefault(slug, 1); + while(fragments.ContainsKey(fragment)) { fragment = slug + "-" + suffix.ToString(CultureInfo.InvariantCulture); + suffix++; } + nextSuffix[slug] = suffix; fragments.Add(fragment, element.Id); } foreach(IElement element in body.QuerySelectorAll("[" + sources.AttributeName + "]")) @@ -56,6 +69,7 @@ public sealed class DocumentOutline link.SetAttribute("href", "#" + target); } } + cancellation.ThrowIfCancellationRequested(); foreach(IElement element in body.QuerySelectorAll("*").Reverse()) { if(!_elements.ContainsKey(element)) diff --git a/src/MarkdownViewr.Windows.Core/Documents/DocumentSourceMap.cs b/src/MarkdownViewr.Windows.Core/Documents/DocumentSourceMap.cs index 649e605..247699a 100644 --- a/src/MarkdownViewr.Windows.Core/Documents/DocumentSourceMap.cs +++ b/src/MarkdownViewr.Windows.Core/Documents/DocumentSourceMap.cs @@ -142,10 +142,12 @@ internal sealed class DocumentSourceMap string html = string.Join('\n', lines.Take(block.Lines.Count).Select(line => line.Slice.ToString())); int? close = null; bool seenTag = false; + var nesting = new HtmlNesting(); var parser = new HtmlParser(new HtmlParserOptions { OnToken = (token, range) => { + nesting.Observe(token); if(!seenTag && token.Type is HtmlTokenType.StartTag or HtmlTokenType.EndTag && token.Name.Length == 2 && token.Name[0] == 'h' && token.Name[1] is >= '1' and <= '6') { @@ -209,11 +211,13 @@ internal sealed class DocumentSourceMap private static HtmlParser CreateSourceParser(Dictionary headingEnds) { int pendingHeading = -1; + var nesting = new HtmlNesting(); return new HtmlParser(new HtmlParserOptions { IsKeepingSourceReferences = true, OnToken = (token, range) => { + nesting.Observe(token); if(token.Name.Length == 2 && token.Name[0] == 'h' && token.Name[1] is >= '1' and <= '6') { if(token.Type == HtmlTokenType.StartTag) diff --git a/src/MarkdownViewr.Windows.Core/Documents/HtmlNesting.cs b/src/MarkdownViewr.Windows.Core/Documents/HtmlNesting.cs new file mode 100644 index 0000000..4c807da --- /dev/null +++ b/src/MarkdownViewr.Windows.Core/Documents/HtmlNesting.cs @@ -0,0 +1,90 @@ +/* + * Copyright (c) 2026 James Maes + * SPDX-License-Identifier: MIT + */ + +using AngleSharp.Dom; +using AngleSharp.Html.Parser; +using AngleSharp.Html.Parser.Tokens; + + +namespace MarkdownViewr.Windows.Core.Documents; + +/// +/// Bounds how deeply authored HTML may nest. Libraries that walk the parsed tree recursively +/// overflow the stack on a few thousand levels, and a stack overflow cannot be caught, so the +/// bound is enforced before any such walk: a cheap count while tokens arrive stops runaway +/// input early, and an exact measurement of the parsed tree applies the real limit. +/// +internal sealed class HtmlNesting +{ + /// The deepest element nesting a document may have; ordinary documents use a few dozen levels. + public const int MaximumDepth = 256; + + /// Start tags left open before parsing stops; an upper bound on depth that keeps the parser's work small. + public const int MaximumOpenTags = 4096; + + private const string Message = "The document nests HTML more deeply than the supported limit."; + + private static readonly HashSet s_neverOpen = new(StringComparer.Ordinal) + { + // Void elements have no content. + "area", "base", "br", "col", "embed", "hr", "img", "input", "link", "meta", "source", "track", "wbr", + // A new one of these closes the previous one, so siblings left unclosed do not nest. + "caption", "colgroup", "dd", "dt", "li", "optgroup", "option", "p", "tbody", "td", "tfoot", "th", "thead", "tr", + }; + + private readonly Stack _open = new(); + + + + /// + /// Counts start tags that are still open. An end tag closes only the tag on top, so the count + /// can exceed the real depth for carelessly closed markup but never falls below it. + /// + public void Observe(HtmlToken token) + { + if(token.Type == HtmlTokenType.StartTag && !s_neverOpen.Contains(token.Name)) + { + _open.Push(token.Name); + if(_open.Count > MaximumOpenTags) + { + throw new ArgumentException(Message, nameof(token)); + } + } + else if(token.Type == HtmlTokenType.EndTag && _open.TryPeek(out string? top) && string.Equals(top, token.Name, StringComparison.Ordinal)) + { + _open.Pop(); + } + } + + + + /// Measures the parsed tree without recursion and rejects one deeper than . + public static void ThrowIfTooDeep(INode root) + { + int depth = 0; + INode? node = root; + while(node is not null) + { + if(node.FirstChild is { } child) + { + node = child; + if(++depth > MaximumDepth) + { + throw new ArgumentException(Message, nameof(root)); + } + } + else + { + // A leaf: climb to the nearest ancestor that has a sibling still to visit. + while(node != root && node!.NextSibling is null) + { + node = node.Parent; + depth--; + } + node = node == root ? null : node!.NextSibling; + } + } + } +} diff --git a/src/MarkdownViewr.Windows.Core/Documents/MarkdownRenderer.cs b/src/MarkdownViewr.Windows.Core/Documents/MarkdownRenderer.cs index a911826..2556df3 100644 --- a/src/MarkdownViewr.Windows.Core/Documents/MarkdownRenderer.cs +++ b/src/MarkdownViewr.Windows.Core/Documents/MarkdownRenderer.cs @@ -6,6 +6,8 @@ using System.Text; using System.Text.RegularExpressions; +using AngleSharp; +using AngleSharp.Css.Parser; using AngleSharp.Dom; using AngleSharp.Html.Dom; using AngleSharp.Html.Parser; @@ -24,6 +26,14 @@ public sealed class MarkdownRenderer /// Bounds parser work before an opened document can consume unbounded memory. public const int MaximumCharacters = 16 * 1024 * 1024; + // The sanitizer's own parser settings: scripting on, so noscript content stays text, and its CSS-aware context. + private static readonly IBrowsingContext s_sanitizerContext = BrowsingContext.New(Configuration.Default.WithCss(new CssParserOptions + { + IsIncludingUnknownDeclarations = true, + IsIncludingUnknownRules = true, + IsToleratingInvalidSelectors = true, + })); + private readonly MarkdownPipeline _pipeline; private readonly MarkdownExtensions _extensions; @@ -100,7 +110,23 @@ public sealed class MarkdownRenderer _ => "", } : ""; string html = sources is null ? RenderDocument(Markdown.Parse(body, _pipeline)) : sources.Render(body, _pipeline, markdown.Length - body.Length, RenderDocument); - return _sanitizer.Sanitize(prefix + html); + return Sanitize(prefix + html); + } + + + + /// + /// Parses once with the sanitizer's own settings, proves the tree is shallow enough for the + /// sanitizer's recursive passes, and only then sanitizes and serializes it. + /// + private string Sanitize(string html) + { + var nesting = new HtmlNesting(); + var parser = new HtmlParser(new HtmlParserOptions { IsScripting = true, OnToken = (token, _) => nesting.Observe(token) }, s_sanitizerContext); + using IHtmlDocument document = parser.ParseDocument("" + html); + HtmlNesting.ThrowIfTooDeep(document.Body!); + _sanitizer.SanitizeDom(document, document.Body!); + return document.Body!.ChildNodes.ToHtml(_sanitizer.OutputFormatter); } @@ -124,21 +150,26 @@ public sealed class MarkdownRenderer var block = (HtmlBlock)node; string html = string.Join('\n', block.Lines.Lines.Take(block.Lines.Count).Select(line => line.Slice.ToString())); var changes = new List<(int Start, int End, string Value)>(); + var nesting = new HtmlNesting(); var parser = new HtmlParser(new HtmlParserOptions { OnToken = (token, range) => { + nesting.Observe(token); if(token.Type is HtmlTokenType.StartTag or HtmlTokenType.EndTag && token.Name is "code" or "pre") { codeDepth = Math.Max(0, codeDepth + (token.Type == HtmlTokenType.StartTag ? 1 : -1)); } else if(token.Type == HtmlTokenType.Character && codeDepth == 0) { - string text = html[range.Start.Index..range.End.Index]; + // An unterminated script or comment reports a range past the end of the block. + int start = Math.Min(range.Start.Index, html.Length); + int end = Math.Clamp(range.End.Index, start, html.Length); + string text = html[start..end]; string formatted = FormatRawText(text, extendSubscript); if(!string.Equals(text, formatted, StringComparison.Ordinal)) { - changes.Add((range.Start.Index, range.End.Index, formatted)); + changes.Add((start, end, formatted)); } } }, @@ -148,13 +179,7 @@ public sealed class MarkdownRenderer { continue; } - var rewritten = new StringBuilder(html); - for(int index = changes.Count - 1; index >= 0; index--) - { - (int start, int end, string value) = changes[index]; - rewritten.Remove(start, end - start).Insert(start, value); - } - string[] lines = rewritten.ToString().Split('\n'); + string[] lines = Rewrite(html, changes).Split('\n'); for(int index = 0; index < block.Lines.Count; index++) { block.Lines.Lines[index].Slice = new Markdig.Helpers.StringSlice(lines[index]); @@ -165,6 +190,28 @@ public sealed class MarkdownRenderer + /// + /// Applies replacements in one forward pass, so the work is linear in the block. Malformed markup + /// can report a text run that starts inside an earlier one; that run is left as authored. + /// + private static string Rewrite(string html, List<(int Start, int End, string Value)> changes) + { + var rewritten = new StringBuilder(html.Length + 64); + int copied = 0; + foreach((int start, int end, string value) in changes) + { + if(start < copied) + { + continue; + } + rewritten.Append(html, copied, start - copied).Append(value); + copied = end; + } + return rewritten.Append(html, copied, html.Length - copied).ToString(); + } + + + /// Reference extension syntax applies only to tokenized visible text, with bounded regex work and unchanged entity spelling. private string FormatRawText(string text, bool extendSubscript) { @@ -198,10 +245,12 @@ public sealed class MarkdownRenderer } else if(node is HtmlBlock block) { + var nesting = new HtmlNesting(); var parser = new HtmlParser(new HtmlParserOptions { OnToken = (token, _) => { + nesting.Observe(token); if(token.Type is HtmlTokenType.StartTag or HtmlTokenType.EndTag && token.Name is "code" or "pre") { codeDepth = Math.Max(0, codeDepth + (token.Type == HtmlTokenType.StartTag ? 1 : -1)); diff --git a/tests/MarkdownViewr.Windows.Core.Tests/HtmlNestingTests.cs b/tests/MarkdownViewr.Windows.Core.Tests/HtmlNestingTests.cs new file mode 100644 index 0000000..f724044 --- /dev/null +++ b/tests/MarkdownViewr.Windows.Core.Tests/HtmlNestingTests.cs @@ -0,0 +1,218 @@ +/* + * Copyright (c) 2026 James Maes + * SPDX-License-Identifier: MIT + */ + +using AngleSharp.Html.Dom; +using AngleSharp.Html.Parser; +using MarkdownViewr.Windows.Core.Documents; + + +namespace MarkdownViewr.Windows.Core.Tests; + +/// +/// Deeply nested markup must be refused as an ordinary error. The renders here run on a thread +/// with a quarter of the default stack, so a recursive walk that the bound failed to stop would +/// end the test run instead of passing by luck. +/// +public sealed class HtmlNestingTests +{ + private const int SmallStackBytes = 256 * 1024; + + + + /// The deepest markup the bound allows renders completely, with stack to spare. + [Fact] + public void Render_NestingAtTheLimit_RendersOnASmallStack() + { + int levels = HtmlNesting.MaximumDepth - 1; + string markup = Repeat("
", levels) + "deepest" + Repeat("
", levels); + string? html = null; + Assert.Null(OnSmallStack(() => html = new MarkdownRenderer().Render(markup))); + using IHtmlDocument parsed = new HtmlParser().ParseDocument(html!); + Assert.Equal(levels, parsed.QuerySelectorAll("div").Length); + Assert.Contains("deepest", html, StringComparison.Ordinal); + Assert.Equal(256, HtmlNesting.MaximumDepth); + } + + + + /// One level past the limit is an argument error, the failure the reader already reports for an unsupported document. + [Fact] + public void Render_NestingOnePastTheLimit_IsRejected() + { + string markup = Repeat("
", HtmlNesting.MaximumDepth) + "too deep"; + Assert.IsType(OnSmallStack(() => new MarkdownRenderer().Render(markup))); + } + + + + /// + /// A few kilobytes of nested elements, in every shape that nests, are refused before any + /// library walks the tree recursively. + /// + [Theory] + [InlineData("
", 3000)] + [InlineData("", 3000)] + [InlineData("", 1500)] + [InlineData("
", 3000)] + [InlineData("
", 2000)] + [InlineData("
  • ", 2000)] + [InlineData("
    s", 1500)] + [InlineData("
    ", 3000)] + public void Render_DeeplyNestedMarkup_IsRejectedBeforeAnyRecursiveWalk(string unit, int count) + { + string markup = Repeat(unit, count) + "x"; + Assert.IsType(OnSmallStack(() => new MarkdownRenderer().Render(markup))); + Assert.IsType(OnSmallStack(() => _ = new DocumentOutline(markup))); + } + + + + /// + /// Nesting far past the limit is stopped by the count kept while tokens arrive, not after the + /// whole input has been parsed: parsing it to the end takes time that grows with its square. + /// + [Fact] + public void Render_RunawayNesting_IsStoppedWhileParsing() + { + string block = Repeat("
    ", 100000) + "x"; + ArgumentException stoppedInBlock = Assert.IsType(OnSmallStack(() => new MarkdownRenderer().Render(block))); + Assert.Equal("token", stoppedInBlock.ParamName); + string inline = "Paragraph " + Repeat("", 100000) + "x"; + ArgumentException stoppedInline = Assert.IsType(OnSmallStack(() => new MarkdownRenderer().Render(inline))); + Assert.Equal("token", stoppedInline.ParamName); + ArgumentException measured = Assert.IsType(OnSmallStack(() => new MarkdownRenderer().Render(Repeat("
    ", 300) + "x"))); + Assert.Equal("root", measured.ParamName); + } + + + + /// Nesting assembled from inline tags in a paragraph, or from many separate blocks, is measured as a whole. + [Theory] + [InlineData("Paragraph ", "", "")] + [InlineData("", "
    ", "\n\n")] + [InlineData("- item ", "", "")] + [InlineData("> quoted ", "", "")] + public void Render_NestingAcrossInlineTagsAndBlocks_IsRejected(string prefix, string unit, string separator) + { + string markup = prefix + string.Concat(Enumerable.Repeat(unit + separator, 400)) + "x"; + Assert.IsType(OnSmallStack(() => new MarkdownRenderer().Render(markup))); + } + + + + /// Nesting hidden in frontmatter rendered as Markdown is bounded like the body. + [Fact] + public void Render_DeepNestingInMarkdownFrontmatter_IsRejected() + { + string markup = "---\n" + Repeat("
    ", 3000) + "\n---\nBody"; + Assert.IsType(OnSmallStack(() => new MarkdownRenderer().Render(markup, FrontmatterMode.Markdown))); + Assert.Null(OnSmallStack(() => new MarkdownRenderer().Render(markup, FrontmatterMode.Hide))); + } + + + + /// Wide documents are not deep: thousands of siblings, unclosed paragraphs, list items and cells render normally. + [Theory] + [InlineData("
    x
    ")] + [InlineData("

    x")] + [InlineData("x
    ")] + [InlineData("xy")] + public void Render_ManySiblings_AreNotMistakenForNesting(string unit) + { + string markup = "

    " + Repeat(unit, HtmlNesting.MaximumOpenTags + 500) + "
    "; + string? html = null; + Assert.Null(OnSmallStack(() => html = new MarkdownRenderer().Render(markup))); + Assert.Contains("x", html, StringComparison.Ordinal); + } + + + + /// List items and table cells left unclosed are siblings, however many there are. + [Fact] + public void Render_ManyUnclosedItemsAndCells_AreNotMistakenForNesting() + { + string markup = "
      " + Repeat("
    • item", 5000) + "
    \n\n" + Repeat("
    ab", 5000) + "
    "; + string? html = null; + Assert.Null(OnSmallStack(() => html = new MarkdownRenderer().Render(markup))); + using IHtmlDocument parsed = new HtmlParser().ParseDocument(html!); + Assert.Equal(5000, parsed.QuerySelectorAll("li").Length); + Assert.Equal(10000, parsed.QuerySelectorAll("td").Length); + } + + + + /// The early count allows exactly its bound of open tags and stops at the next one. + [Theory] + [InlineData(HtmlNesting.MaximumOpenTags, false)] + [InlineData(HtmlNesting.MaximumOpenTags + 1, true)] + public void Observe_OpenTagsAtAndPastTheBound_StopsExactlyPastIt(int count, bool stopped) + { + Assert.Equal(stopped, Record.Exception(() => Tokenize(Repeat("
    ", count))) is ArgumentException); + Assert.Equal(4096, HtmlNesting.MaximumOpenTags); + } + + + + /// + /// The early count never falls below the real depth: an end tag closes only the tag on top, + /// so an end tag the parser would ignore cannot be used to hide nesting from the count. + /// + [Theory] + [InlineData("
    ")] + [InlineData("
    ")] + [InlineData("

    ")] + public void Observe_EndTagThatDoesNotCloseTheOpenTag_DoesNotReduceTheCount(string unit) + => Assert.IsType(Record.Exception(() => Tokenize(Repeat(unit, HtmlNesting.MaximumOpenTags + 1)))); + + + + ///

    Properly closed elements, void elements and self-closing siblings never accumulate. + [Theory] + [InlineData("
    x
    ")] + [InlineData("
    ")] + [InlineData("

    x

  • y
    		• z