fix(security): resolve CodeQL alerts on develop→staging - #37
Merged
Conversation
- generate-random-port: replace modulo-biased random port selection with unbiased rejection sampling over crypto.getRandomValues - convert-xml-json: escape double/single quotes in generated XML attribute values to prevent attribute injection - format-xml: fix comment-detection regex to match comments spanning multiple lines - generate-svg-placeholder: escape backgroundColor/textColor before interpolating them into SVG attributes rendered via dangerouslySetInnerHTML, closing a DOM XSS vector - base64-file-panel: rebuild the download link's data URL from the validated mimeType/base64 parts instead of the raw pasted text - suppress the remaining js/xss-through-dom findings on the well-formedness DOMParser.parseFromString calls, which only read tagName/attributes/textContent into plain values and never insert parsed nodes into the live DOM
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
CodeQL still flagged the previous fix (rebuilding the data: URL from validated parts) because the taint tracker follows the mimeType/base64 match groups through the template literal regardless of the hardcoded scheme prefix. Decode the payload into a Blob and download it via URL.createObjectURL instead, matching the pattern already used by DownloadButton — the anchor's href is then a browser-generated blob: URL with no path for user-controlled text to reach it at all.
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
2 participants
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.
Summary
CodeQL was failing on #2 (develop → staging) with 8 new alerts (7 high, 1 medium). This fixes the real issues and documents the two remaining well-formedness parses as non-exploitable.
js/biased-cryptographic-random(generate-random-port): modulo oncrypto.getRandomValuesbiased low ports; replaced with rejection sampling.js/incomplete-html-attribute-sanitization(convert-xml-json):escapeXmldidn't escape"/', letting a JSON value break out of a generated XML attribute; now escapes them.js/bad-tag-filter(format-xml): the comment-detection regex used.withoutdotAll, so XML comments spanning multiple lines weren't recognized as comments (broke indentation, and is exactly the kind of filter bypass CodeQL flags); fixed with[\s\S].js/xss-through-dom× 2 (svg-placeholder-panel via generate-svg-placeholder):backgroundColor/textColorwere interpolated unescaped into SVG attributes that get rendered viadangerouslySetInnerHTML— a real DOM XSS (e.g."/><script>...). Now escaped liketextalready was.js/xss-through-dom(base64-file-panel): the download link'shrefwas set directly from the raw pasted text; rebuilt it from the already-validatedmimeType/base64parts so the URL scheme can never be attacker-controlled.js/xss-through-dom× 3 (convert-xml-json ×2, format-xml ×1, bothisWellFormedwell-formedness checks): the parsedDOMParserDocumentis only used to readtagName/attributes/textContentinto plain JSON/text — it's never inserted into the live DOM, so there's no actual sink downstream (output is rendered as a React text child, which auto-escapes). Suppressed with an inlinecodeql[js/xss-through-dom]comment explaining why, per GitHub's suppression mechanism.Regression tests were added for every fix (attribute escaping, SVG color escaping, multi-line XML comments, rejection-sampling behavior via a mocked
crypto.getRandomValues, and the rebuilt download href).Test plan
pnpm test— 754 tests passing (5 new)pnpm lintpnpm build(astro check + build)