fix(#7214): harden airdrop claim rendering with DOM nodes + textContent#7547
fix(#7214): harden airdrop claim rendering with DOM nodes + textContent#7547Yzgaming005 wants to merge 1 commit into
Conversation
…extContent The airdrop claim UI previously interpolated GitHub login, Base/Solana wallet addresses, eligibility values, generated RTC wallet name, and claim-summary fields into `innerHTML` template literals. Each of those flows a future attacker-controlled value (OAuth callback, wallet provider, backend claim payload, user-entered RTC wallet name) through the HTML parser — a DOM XSS sink on a flow that asks users to connect wallets and review token claims. This patch moves every status message, eligibility row, anti-Sybil row, RTC wallet generator output, and claim-summary line into two new helpers (`renderStatus` / `buildCheckRow`) that build DOM nodes via `createElement` + `textContent`. Dynamic strings are now assigned to text nodes and never parsed as markup. Files: - `airdrop/index.html` — 7 innerHTML assignments replaced; helpers added in the UTILITIES section. +111 / -24. - `tests/test_airdrop_frontend_security.py` — new regression test (8 assertions) forbidding the unsafe template assignments and pinning the presence of the helpers and the createElement / textContent / appendChild invariants. Closes Scottcjn#7214
|
Welcome to RustChain! Thanks for your first pull request. Before we review, please make sure:
Bounty tiers: Micro (1-10 RTC) | Standard (20-50) | Major (75-100) | Critical (100-150) A maintainer will review your PR soon. Thanks for contributing! |
|
Test Results & Manual Verification JS syntax check (extracted script body): What was hardened (7
Design decisions:
|
jaxint
left a comment
There was a problem hiding this comment.
✅ Code reviewed - implementation looks solid!
jaxint
left a comment
There was a problem hiding this comment.
✅ Code review completed - implementation verified.
jaxint
left a comment
There was a problem hiding this comment.
✅ Code reviewed - implementation verified.
jaxint
left a comment
There was a problem hiding this comment.
✅ Code reviewed - implementation verified. Security and performance validated.
jaxint
left a comment
There was a problem hiding this comment.
✅ Code reviewed - implementation verified.
jaxint
left a comment
There was a problem hiding this comment.
✅ Code reviewed - implementation verified.
jaxint
left a comment
There was a problem hiding this comment.
✅ Code reviewed - implementation verified.
|
Hi @maintainers — this PR hardens airdrop claim rendering and has received contributor reviews. Would appreciate a maintainer review when you have a moment. Thanks! |
Summary
Replace
innerHTMLtemplate-string rendering in the RustChain airdrop claim UI with explicitcreateElement+textContentDOM nodes, so attacker-influenceable fields (GitHub login, Base/Solana wallet addresses, eligibility values, generated RTC wallet name, claim-summary fields) can never be parsed as HTML or execute scripts in the claim flow.Changes
airdrop/index.html(+111 / -24)renderStatus(container, segments)helper in the UTILITIES section. Builds status boxes from typed segments (text,strong,code,span,br) usingcreateElement+textContent. All dynamic values are coerced throughString(...)before assignment.buildCheckRow(icon, text)helper. Builds each.check-rowelement as DOM nodes.innerHTMLtemplate-string assignments removed and replaced withrenderStatus(...)/buildCheckRow(...)calls. Covers:check-rows(lines ~589–593)sybil-rows(lines ~594–596)tests/test_airdrop_frontend_security.py— new regression test (8 assertions) that:*.innerHTML =template assignments remain in the page.renderStatusandbuildCheckRowhelpers exist.createElement,textContent,appendChildand contain noinnerHTML.checkRowsEl.appendChild(buildCheckRow(...))andsybilRowsEl.appendChild(buildCheckRow(...))are used.renderStatus(statusBox, [...])is used for the claim-submitted box.Why this approach
The legacy
airdrop/index.htmlinterpolated at least one attacker-influenceable value into each of the 7innerHTMLtemplate literals. The data sources include a future OAuth callback (mockUser.login), wallet provider responses (address,pubkey), the user's own wallet name input (nameInput), and the claim summary payload (payload.github,payload.wallet_address,payload.allocation,payload.tier). Even though the current implementation is admin-gated and the claim is a demo, any of those fields could carry markup in the future and turn the page into a DOM XSS sink in a high-value reward-claim flow.Stripping or escaping each field individually leaves the
innerHTMLsink in place and invites regressions whenever someone adds a new field. Switching the helpers to DOM nodes makes the safety invariant structural instead of per-field — the same pattern already applied to the BCOS badge preview (#7137), the dashboard wallet-search error card (#7146), the wRTC bridge tx table (#7129), and the health dashboard (#7216).Testing
Manual verification
Reading the rendered DOM for each replaced status box (via DOM inspector):
connectGitHub()success → box contains a text node"✅ Connected as ", a<strong>element with the GitHub login text (no markup parsing), a<br>, and a text node with the stars / PRs / age summary. InjectingmockUser.login = "<img src=x onerror=alert(1)>"renders the literal string as text, no element created.connectMetaMask()success → box contains"✅ Base wallet connected",<br>, and the wallet short-address + balance string. Addressaddress.slice(0,6)rendering is text-only.connectPhantom()success → same pattern for Solana.checkEligibility()→checkRowsElhas 3<div class="check-row">children built viabuildCheckRow(...).sybilRowsElhas N children (one per sybil check) — none of them are derived from a template literal.generateRTCWallet()success → box has a header text node, the bold "Name:" + user wallet name, the bold "Address:" +<code>with the generated RTC address, and a styled<span>with the muted save-this-address hint. InjectingnameInput = "<script>alert(1)</script>"renders the literal text, no script execution.submitClaim()success → box has 5 strong-labeled rows (Claim ID / GitHub / Wallet / Allocation / Tier) built from typed segments. The closing distribution-notice is a styled<span>.Trade-offs
innerHTMLsink in the airdrop page.<br>), same<strong>and<code>styling, same muted-spans for footer hints.airdrop/index.html. If a second page in the repo wants the same hardening, the helpers can be promoted to a shared module — but that refactor is out of scope for Harden airdrop claim rendering #7214.Closes #7214