Repository navigation
fix: only an http or https package address reaches src, parsed - #72
Merged
Merged
Conversation
An address from a link, a text field or the cmi5 launch was set as the element's src as written. The element only fetches it, but CodeQL cannot know that: it flagged each place under js/xss, js/xss-through-dom and js/client-side-unvalidated-url-redirection, and 22 alerts dismissed as false positives came back whenever a line around them moved. - The demo's player page, cmi5 page and app take the address through packageUrl (page-common.js), and the embed page through its own copy: resolved with new URL, http and https only, the parsed URL's href set. Anything else is refused with a message and plays nothing. - startCmi5 gives the element only an http or https address, resolved against the page's (webAddress takes a base); anything else is an unsafe-src event, and the next source is tried. - CodeQL's model of new URL(x) carries taint to search, hash and searchParams only, so the href is clean to all three queries. 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.
Closes #71.
Every place that sets the element's
srcfrom a link, a text field or the cmi5 launch now resolves the address withnew URL, accepts onlyhttp:andhttps:, and sets the parsed URL'shref. Anything else is refused with a message and plays nothing. This is the fix CodeQL suggested on alert 35, applied to each place. The preview page gets the same in #70.Changes
apps/demo/demo/page-common.js):packageUrl(src), besideforeignHost, returns the parsed URL ornull.?src=), the cmi5 page (?src=,launchParameters) and the app (?src=, launch links) take the address through it.That is not an http or https address: …for anything else.packages/embed/site/embed.js): its ownpackageUrl.?src=that is nothttporhttpsis refused withThe package address is not an http or https URL., posted to the parent aserrorwithcode: 'refused', as other refusals are.data:URL was taken for another site's package and played untrusted.packages/cmi5/src/index.ts):startCmi5gives the element only anhttporhttpsaddress, resolved against the page's address (url).unsafe-srcevent, likeunsafe-return-url, and the next source is tried: the address's?src=, thenlaunchParameters.webAddress(value, base?)takes the base.session.srcis now the absolute address the element was given.docs/cmi5.mdlistunsafe-srcand the rule; the embed README says whatsrc=takes; AGENTS.md records the rule and why, so the next place that setssrcfollows it instead of dismissing another alert.Why CodeQL accepts this
search,hashandsearchParamsonly, so the parsed URL'shrefis clean to all three queries (js/xss,js/xss-through-dom,js/client-side-unvalidated-url-redirection).startsWith('https:'), satisfiesjs/xssonly.srcfrom a parsed URL, and CodeQL never flagged it.Testing
javascript:package in the address and inlaunchParameters, each reported asunsafe-srcand never given to the element;webAddresswith a base.javascript:and adata:package refused with the message, and nothing set assrc.javascript:link shows the message on the home screen, sets nosrc, and raises no page error.srcjavascript:alert(1)/demo/content/quiz.h5phttp://localhost:…/demo/content/quiz.h5pjavascript:alert(1)/demo/content/quiz.h5phttp://localhost:…/demo/content/quiz.h5pNo page errors and no dialogs in any of the four.
Release
Both are security fixes under
docs/compatibility.md: they refuse something hostile.unsafe-srcevent is new in its types; aswitchoverCmi5Eventwith no default branch must handle it.Their notes should list the fix first.
Not changed: the element itself, which still takes any URL a host sets, and the three
js/incomplete-html-attribute-sanitizationalerts incontent-check.ts, which are a different rule.🤖 Generated with Claude Code