fix: secure SSL handling and Web3 provider injection timing#3438
fix: secure SSL handling and Web3 provider injection timing#3438jim-daf wants to merge 2 commits into
Conversation
…h handler.cancel() to reject invalid certificates by default Addresses AlphaWallet#2286
Use WebViewCompat.addDocumentStartJavaScript() to inject the ethereum provider before any page scripts run. This ensures window.ethereum is available when sites try to detect it, fixing the @metamask/detect-provider failure on sites with strict CSP like exchange.idex.io. Addresses AlphaWallet#2286
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Addresses WebView-layer security and reliability issues by rejecting invalid SSL certificates and ensuring window.ethereum is injected early enough for strict-CSP sites to detect the provider reliably.
Changes:
- Reject SSL certificate errors in TokenScript WebViews by cancelling SSL error handling instead of proceeding.
- Inject the Web3 provider script at document start (when supported) using AndroidX WebKit for reliable provider availability pre-page-scripts.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| app/src/main/java/com/alphawallet/app/web3/Web3View.java | Adds document-start provider injection via WebViewCompat gated by WebViewFeature.DOCUMENT_START_SCRIPT. |
| app/src/main/java/com/alphawallet/app/web3/Web3TokenView.java | Cancels on SSL errors (secure default) and adds explanatory comments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // handler.proceed() was removed to prevent accepting untrusted certificates. | ||
| // For development/testing with self-signed certs, consider using | ||
| // network_security_config.xml instead. | ||
| handler.cancel(); // Ignore SSL certificate errors |
There was a problem hiding this comment.
The inline comment is now inaccurate: handler.cancel() rejects the SSL certificate error rather than ignoring it. Update the comment to reflect the actual behavior (e.g., 'Reject SSL certificate errors').
| handler.cancel(); // Ignore SSL certificate errors | |
| handler.cancel(); // Reject SSL certificate errors |
| if (WebViewFeature.isFeatureSupported(WebViewFeature.DOCUMENT_START_SCRIPT)) | ||
| { | ||
| String providerJs = webViewClient.getJsInjectorClient().providerJs(getContext()); | ||
| Set<String> origins = new HashSet<>(Collections.singletonList("*")); |
There was a problem hiding this comment.
This can be simplified to avoid the extra allocation and verbosity (e.g., use an immutable singleton set directly). That keeps the intent clearer and reduces unnecessary object creation.
| { | ||
| String providerJs = webViewClient.getJsInjectorClient().providerJs(getContext()); | ||
| Set<String> origins = new HashSet<>(Collections.singletonList("*")); | ||
| WebViewCompat.addDocumentStartJavaScript(this, providerJs, origins); |
There was a problem hiding this comment.
The origin rule set is currently wildcarded (*), which injects the provider into every loaded origin. If the intended security model is to expose wallet APIs only to specific dApp origins (or only when a user explicitly connects), consider restricting origins to an allowlist (or otherwise gating injection) to reduce the attack surface from arbitrary web content.
Security Fixes (Issue #2286)
Resolves #2286
Problem
Two security/functionality issues in the WebView layer:
Insecure SSL handling (
Web3TokenView.java):onReceivedSslError()calledhandler.proceed(), blindly accepting invalid SSL certificates. This enables man-in-the-middle attacks — critical for a crypto wallet.Web3 provider injection timing (
Web3View.java): Thewindow.ethereumprovider was injected inonPageStarted()viaevaluateJavascript(). While this bypasses CSP, it has a timing problem: page scripts can execute before the asyncevaluateJavascript()completes, causing@metamask/detect-providerto fail with "Unable to detect window.ethereum" on sites likeexchange.idex.io.Changes Made
app/src/main/java/com/alphawallet/app/web3/Web3TokenView.javahandler.proceed()withhandler.cancel()inonReceivedSslError()to reject invalid certificates by default.app/src/main/java/com/alphawallet/app/web3/Web3View.javaWebViewCompat.addDocumentStartJavaScript()ininit()to inject the Web3 provider script at document start, before any page scripts run.evaluateJavascript()calls inonPageStarted()are kept for the init/configuration script that requires wallet address and chain ID context.WebViewFeature.DOCUMENT_START_SCRIPT) for backward compatibility.Testing Notes
window.ethereumreliably, since the provider is injected synchronously at document start.evaluateJavascript()fallback inonPageStarted()remains for the init script and for WebView implementations that don't supportDOCUMENT_START_SCRIPT.