fix(web3): sanitize chain RPC URL before injecting into in-app browser (UXSS)#3440
Open
jim-daf wants to merge 1 commit into
Open
fix(web3): sanitize chain RPC URL before injecting into in-app browser (UXSS)#3440jim-daf wants to merge 1 commit into
jim-daf wants to merge 1 commit into
Conversation
…s (UXSS) Resolves AlphaWallet#2686 JsInjectorClient.loadInitJs splices the configured RPC URL into a JavaScript template via String.format and the result is injected into every page that loads in the in-app dApp browser. A malicious dApp could exploit this by calling wallet_addEthereumChain with an RPC URL such as https://rpc.example/x"+alert(document.domain)+" After the user accepts the new chain, the URL is persisted to the network database and used as-is on every subsequent page load, breaking out of the RPC-URL string literal in the template and giving the attacker a stored, universal XSS that runs in the origin of any site visited in the browser (including unrelated dApps and major sites). This change adds a sanitizeRpcUrl helper that: * Rejects empty/null URLs. * Rejects URLs containing any character with meaning inside a JS string literal (quotes, backslash, angle brackets, control characters). * Parses the URL with HttpUrl.parse and rejects anything that is not a valid http(s) URL. * Returns the canonical normalized URL string and re-checks it for unsafe characters before returning. setChainId and setTSChainId now run the resolved RPC URL through this helper before storing it. If the URL is unsafe, an empty string is used, which makes the injected provider non-functional for that chain but prevents the UXSS payload from executing. Defensive: loadInitJs also coalesces a null rpcUrl to an empty string so String.format never sees null after the change.
cda0e83 to
28771ec
Compare
There was a problem hiding this comment.
Pull request overview
This PR mitigates a stored universal-XSS (UXSS) vector in AlphaWallet’s in-app dApp browser by sanitizing the active chain’s RPC URL before it is interpolated into injected JavaScript.
Changes:
- Introduces
sanitizeRpcUrl(String)to validate/normalize RPC URLs and reject inputs unsafe for JS string literal splicing. - Applies RPC URL sanitization when setting
chainIdfor both normal and TokenScript injection paths. - Ensures
loadInitJsnever passes anullRPC URL intoString.format.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Sanitize the chain RPC URL before injecting it into in-app browser pages
Resolves #2686 - Universal XSS via
wallet_addEthereumChainRPC URL.The vulnerability
JsInjectorClient.loadInitJs(...)builds the JavaScript that is injected intoevery page loaded in the AlphaWallet dApp browser by interpolating the active
chain's RPC URL into a template with
String.format:The RPC URL is whatever value the network record holds, which a malicious dApp
can control by calling
wallet_addEthereumChainwith a payload like:After the user accepts the new chain (a benign-looking confirmation), every
subsequent page loaded in the in-app browser is poisoned with the attacker's
JavaScript, executed in the origin of that page — a classic stored UXSS.
What this PR does
app/src/main/java/com/alphawallet/app/web3/JsInjectorClient.java:sanitizeRpcUrl(String)which:string literal (
",',\,<,>, CR, LF, tab, other controlbytes);
HttpUrl.parse(...)and rejects anything thatisn't a valid
http/httpsURL;characters in case the parser decoded any.
EthereumNetworkRepository.getDefaultNodeURL(chainId)through that helperinside both
setChainIdandsetTSChainIdso the field stored on theclient is always safe to splice.
rpcUrlto""inloadInitJssoString.formatneversees
nullafter the change.