Escape the prose-derived values that land in a title attribute - #1
Open
GetBeholder wants to merge 1 commit into
Open
Escape the prose-derived values that land in a title attribute#1GetBeholder wants to merge 1 commit into
GetBeholder wants to merge 1 commit into
Conversation
Every value on a slot originates in prose and arrives through the extractor,
so by the time it reaches markup it is attacker-influenced. Two of them
reached a title="…" attribute unescaped:
- the colour word, interpolated raw by colorTitle
- the damage word, whenever it fell outside the known tiers and damageMeta
echoed the raw prose word back as its label
A crafted value closes the attribute and adds one of its own; rendering a
payload into a real DOM and asking the parser confirms an injected event
handler becomes an actual attribute node. Every sibling label already escaped,
so this was an oversight rather than a choice.
The new test renders eight prose-derived fields plus the character name with a
breakout payload, in both views, and requires the raw payload never survives
into the markup. It fails against the previous code.
Output for well-formed values is unchanged.
Co-Authored-By: Claude <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
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.
Same fix as GetBeholder/Beholder-ME#1 — this renderer carries the identical three sites.
colorTitleinterpolated the colour word straight into atitle="…"attribute, and bothdmgTitlesites did the same with the damage word whenever it fell outside the known tiers anddamageMetaechoed the raw prose word back as its label.Every value on a slot comes from prose by way of the extractor, so a crafted value can close the attribute and add one of its own. Confirmed by rendering the payload into a real DOM and asking the parser: an injected
onmouseoverbecomes an actual attribute node, not text. Every sibling label (colorSwatch,colorLabel,dmgLabel) already escaped — these three were an oversight.Scope: three lines in
paperdoll.js. No bundle to rebuild here; modules load directly.Test:
test/paperdoll_escaping.test.jsrenders eight prose-derived fields plus the character name with a breakout payload, in both views. Verified it fails against the previous code. Suite: 222 pass, 0 fail.Output for well-formed values is unchanged.