Repository navigation
[Fix] Sanitize XHTML and encode text from ReqIF files before rendering - #87
Open
samatstariongroup wants to merge 2 commits into
Open
samatstariongroup wants to merge 2 commits into
samatstariongroup wants to merge 2 commits into
Conversation
fixes #57 Content of a ReqIF file was rendered with `new MarkupString(...)` in 21 places, so a crafted file could inject script into the page: XHTML values, but also long names, identifiers and text decoded from XHTML. - New SafeMarkup helper: FromXhtml removes namespace prefixes and sanitizes with HtmlSanitizer against an allow-list of the XHTML modules ReqIF uses (no scripts, event handlers, javascript:/data: links, class or id attributes, positioning styles); FromText HTML-encodes plain text. - ExtractDisplayName returns a safe MarkupString in every case; all call sites use it or SafeMarkup instead of wrapping content themselves. - The relation matrix uses the plain-text ExtractDisplayText for labels. - Tests: SafeMarkup unit tests (formatting kept, attacks removed) and component tests that render TestData/xss-attempts.reqif, a ReqIF file with script, event handler, javascript:, SVG and overlay payloads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ests - SafeMarkup.ToPlainText extracts the visible text of an XHTML value without parsing it as XML; ExtractDisplayText uses it, so a value that is not well-formed (for example plain text without an element) no longer makes the relation matrix or the sidebar fail. - ExtractDisplayName falls back to the identifier when nothing visible is left (empty value, or XHTML with only removed content). - The Target column of the spec relations list linked to the source. - Tests for the specification tree, the "Selected Value" card, the XHTML default value page, embedded object captions, relation matrix labels, CSS url()/expression() in allowed properties, elements removed with their content, allowed elements, link schemes (relative links are kept) and ToPlainText. - CLAUDE.md: rule for rendering content from a ReqIF file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Prerequisites
Description
Fixes #57.
Content of an uploaded ReqIF file was rendered with
new MarkupString(...)in 21 places, so a crafted file could inject script into the page of whoever opened it. This was not limited to XHTML values: long names, identifiers, default values and text decoded from XHTML (<script>becoming live markup again) were also rendered as raw HTML.One safe path for file content (
Utilities/SafeMarkup.cs)FromXhtml: removes namespace prefixes (xhtml:, other prefixes, default namespace) and sanitizes with HtmlSanitizer against an allow-list of the XHTML modules ReqIF uses: text, hypertext, list, presentation, edit and table. Removed: scripts and other non-text elements (with their content), event handler attributes,javascript:anddata:links (also entity-encoded or space-prefixed),classandidattributes, and positioning or layering CSS (so file content cannot cover the page).url()andexpression()in styles are removed.FromText: HTML-encodes plain text.ToPlainText: the visible text of an XHTML value for labels. Unlike ReqIFSharp'sExtractUnformattedTextFromValueit does not parse the value as XML, so values that are not well-formed no longer throw (this could make the relation matrix and the sidebar fail).ExtractDisplayNamereturns a safeMarkupStringin every case and falls back to the identifier when nothing visible is left; all call sites use it orSafeMarkupinstead of wrapping content themselves.Differences from the acceptance criteria of #57
objectis not on the allow-list. Objects (embedded pictures) are shown by their fallback text, which is what users saw before: browsers never rendered the prefixed<xhtml:object>element. Showing pictures inline is [Feature]: Show pictures inline in rich text #71.#anchor,other.html) are kept: they stay within the viewer, and an unknown address shows the not-found page.Also
Testing
SafeMarkupTestFixture: legitimate formatting kept (paragraphs, bold, lists, tables, safe and relative links), each attack removed, plain text extraction.XssAttemptsTestFixturerendersTestData/xss-attempts.reqif, a ReqIF file with script, imageonerror,javascript:link, SVGonloadand fixed-position overlay payloads in texts, long names, identifiers and the document title, through the attribute values grid, the "Selected Value" card, the specification tree, the XHTML default value page and the embedded objects section.xss-attempts.reqif: nothing executes (each payload would setwindow.__xss), markup in names shows as text, the overlay stays inside its cell, legitimate formatting renders.SONAR_TOKENsecret is renewed ([Feature]: Fix CI: renew SONAR_TOKEN and re-enable CodeQL #59).🤖 Generated with Claude Code