fix(security): abort WebView load on SSL certificate errors (CWE-295)#4
fix(security): abort WebView load on SSL certificate errors (CWE-295)#4jim-daf wants to merge 1 commit into
Conversation
…mentActivity WebViewClient.onReceivedSslError previously called handler.proceed(), which accepts any invalid TLS certificate without verification (CWE-295: Improper Certificate Validation). Because this WebView loads the bKash checkout flow, a network attacker on the same network could MITM the payment session and harvest credentials. Replace handler.proceed() with handler.cancel() so the request is dropped whenever the certificate fails platform validation. Resolves SaadAAkash#1
jim-daf
changed the title
jim-daf
force-pushed
the
fix/security-issue-1
branch
from
d8b13b6 to
d08f4ae
Compare
There was a problem hiding this comment.
Pull request overview
This PR addresses CWE-295 by changing the WebView SSL error handling in the bKash checkout flow to fail closed, preventing the app from proceeding on invalid TLS certificates during payment.
Changes:
- Update
onReceivedSslErrorto callhandler.cancel()instead ofhandler.proceed(). - Add inline security rationale documenting why the request must be aborted for payment flows.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| override fun onReceivedSslError(view: WebView?, handler: SslErrorHandler, error: SslError?) { | ||
| handler.proceed() | ||
| // CWE-295: do NOT proceed on invalid certificates. This is a | ||
| // payment WebView; silently trusting any cert (or even | ||
| // prompting the user) lets a network attacker MITM the | ||
| // checkout flow. Abort the request instead. | ||
| handler.cancel() | ||
| } |
There was a problem hiding this comment.
After switching to handler.cancel(), the page load will be aborted on SSL errors, but this WebViewClient doesn’t handle the failure path (e.g., hide loadingProgressBar or show an error/finish the activity). Since onPageFinished won’t run, the progress bar can remain visible indefinitely and the user gets a stuck payment screen. Consider hiding the progress UI and presenting a clear failure state when onReceivedSslError triggers.
Security fix - issue #1 (CWE-295)
Resolves #1
Root cause
BkashPaymentActivity.initBkashWebViewClientoverridesonReceivedSslErrorand unconditionally callshandler.proceed():That tells the WebView to trust any certificate the server
presents - expired, self-signed, hostname-mismatched, or attacker-
issued. Because this Activity loads the bKash checkout flow, a network
attacker on the same Wi-Fi (coffee shop, hotel, hostile ISP) can MITM
the TLS connection and read or rewrite the payment request and
response. Google Play also flags this exact pattern and will warn /
block release uploads.
What this PR changes
One file,
app/src/main/java/ninja/saad/bkashdemo/features/bkash/BkashPaymentActivity.kt:onReceivedSslErrornow callshandler.cancel()instead ofhandler.proceed(), so the request is aborted whenever thecertificate fails the platform's validation.
Why not show an "ignore?" dialog (the snippet from the issue)
The original issue suggests an
AlertDialogasking the user whetherto continue. That is not appropriate for a payment screen: a
non-technical bKash customer faced with "Continue anyway?" will tap
yes, defeating the protection. The correct behaviour is to fail
closed; if a user really has a broken-clock or corporate-MITM
environment, they will see the connection fail and can take it up
with their network admin.
Scope
Only the SSL handler is touched. Existing JS interface, navigation
handling, and progress-bar logic are unchanged.