fix: sanitize AI review HTML and verify iframe postMessage origin#3546
Open
TheChosenOne-Sunyuchen wants to merge 1 commit into
Open
Conversation
Addresses the two security findings reported in tscircuit#3376. 1. ViewAiReviewView: marked.parse() output was passed straight to dangerouslySetInnerHTML. Since the markdown comes from the registry's ai_review_text response and marked does not sanitize by default, any HTML/script in that response executed in the user's DOM. The parsed HTML is now run through DOMPurify before rendering, which strips scripts and event-handler attributes while keeping safe markup (links, formatting). 2. RunFrameWithIframe: the message handler had no origin/source check and replied with postMessage(..., "*"), so any embedding page could send "runframe_ready_to_receive" and receive runFrameProps. The handler now only responds to messages whose source is our own iframe, and targets the iframe's resolved origin instead of "*". Origin resolution is extracted into resolveIframeTargetOrigin() with unit tests.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
1 participant
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.
Fixes the two security issues reported in #3376. Thanks to @Noa-Lia for the report — I independently verified both findings against the current code and implemented the fixes below.
1. Stored/reflected XSS in
ViewAiReviewViewlib/components/AiReviewDialog/ViewAiReviewView.tsxpassedmarked.parse()output directly intodangerouslySetInnerHTML. The markdown comes from the registry'sai_review_textresponse, andmarkeddoes not sanitize by default, so any HTML/script in that response executed in the user's DOM.Fix: run the parsed HTML through
DOMPurify.sanitize()before rendering. Safe markup (links, formatting) is preserved;<script>and event-handler attributes (onerror, etc.) are stripped.2.
postMessagedata leak inRunFrameWithIframelib/components/RunFrameWithIframe/RunFrameWithIframe.tsxlistened formessageevents with noorigin/sourcecheck and replied withpostMessage(..., "*"). Any page embedding the iframe could send{ runframe_type: "runframe_ready_to_receive" }and receiverunFramePropsback.Fix: only respond when
event.sourceis our own iframe'scontentWindow, and target the iframe's resolved origin instead of"*". The embed contract documented in the README is preserved. Origin resolution is extracted intoresolveIframeTargetOrigin()(handles absolute and relativeiframeUrl, returnsnullon unparseable input) with unit tests.Verification
bunx tsc --noEmit— passesbun run format:check(biome) — passesbun test— 23 pass / 0 fail (added 5 tests forresolveIframeTargetOrigin)<script>/onerrorpayloads while keeping legitimate links.