Skip to content

Comments

BOM_1.6.5#287

Merged
jakubuid merged 28 commits intomasterfrom
develop
Feb 11, 2026
Merged

BOM_1.6.5#287
jakubuid merged 28 commits intomasterfrom
develop

Conversation

@jakubuid
Copy link
Collaborator

No description provided.

jakubuid and others added 28 commits January 30, 2026 13:13
- Add detailed logging for WebView -> Wallet JS interface communication
- Display payment result info (txId, token amount) on success screen
- Improve log messages with clear markers and appropriate log levels

Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Add prefill query parameter to IC URL with Base64-encoded user data
(fullName, dob) to pre-populate form fields in the WebView. The prefill
data is built based on the schema's required fields array.

Also removes temporary IC preview URL workaround.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Mark fields property as deprecated in favor of using url for WebView-based
data collection or schema to parse field requirements.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Make IC WebView fullscreen instead of fixed height
- Add floating close button with dark theme styling
- Close button returns to Intro screen instead of canceling
- Remove loading flash when canceling payment flow
- Open external links (Terms, Privacy) in mobile browser
- Add pobAddress to prefill fields

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ationTest

Update test to parse collectDataAction.schema to determine what data
to collect for confirmPayment instead of using hardcoded values based
on deprecated fields.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
feat: Add WebView-based Information Capture for payments
@claude
Copy link

claude bot commented Feb 11, 2026

Claude finished @jakubuid's task in 2m 6s —— View job


Code Review Complete

PR Stats: 16 files, +683/-132 lines. Version bump to 1.6.5 with Payment enhancements.

Summary: Major payment flow enhancements including WebView-based information capture with prefill, schema parsing, and improved payment result handling. Found 5 security/reliability issues requiring attention.


Found 5 issue(s)

Issue 1: JavaScript interface exposed without @RequiresApi protection

ID: paymentroute-js-interface-api-7a29
File: sample/wallet/src/main/kotlin/com/reown/sample/wallet/ui/routes/dialog_routes/payment/PaymentRoute.kt:1910
Severity: HIGH
Category: security

Context:

  • Pattern: addJavascriptInterface called at line 1910 without API level check
  • Risk: Pre-API 17 (Android 4.2), addJavascriptInterface has critical RCE vulnerability (CVE-2012-6636) allowing arbitrary code execution via reflection
  • Impact: While minSdk=23 mitigates this, explicit annotation documents safety and prevents accidental backports
  • Trigger: If code is copied to project with lower minSdk, or minSdk is reduced

Recommendation:

// Add to class or method containing addJavascriptInterface call:
@androidx.annotation.RequiresApi(android.os.Build.VERSION_CODES.JELLY_BEAN_MR1)
private fun WebViewDataCollectionContent(...) {

Issue 2: Missing WebView input validation on URL parameter

ID: paymentroute-webview-url-validation-3f12
File: sample/wallet/src/main/kotlin/com/reown/sample/wallet/ui/routes/dialog_routes/payment/PaymentRoute.kt:1981
Severity: MEDIUM
Category: security

Context:

  • Pattern: WebView loads URL from icWebViewUrl (line 1981) without validating it's from trusted domain
  • Risk: If backend is compromised or returns malicious URL, wallet loads arbitrary content with JS access to AndroidWallet interface
  • Impact: Phishing, data exfiltration via JS bridge, malicious data capture forms
  • Trigger: Backend compromise, DNS hijack, or API manipulation

Recommendation:

// Before loading URL at line 1981, add domain validation:
private fun isAllowedPaymentDomain(url: String): Boolean {
    val allowedHosts = setOf("pay.walletconnect.com", "pay.reown.com")
    return try {
        val host = Uri.parse(url).host
        allowedHosts.any { host?.endsWith(it) == true }
    } catch (e: Exception) { false }
}

// In WebView factory:
if (!isAllowedPaymentDomain(url)) {
    loadError = "Untrusted payment domain"
    return@AndroidView
}
loadUrl(url)

Issue 3: Hardcoded test credentials in production code

ID: ethaccountdelegate-hardcoded-pii-8c45
File: sample/wallet/src/main/kotlin/com/reown/sample/wallet/domain/account/EthAccountDelegate.kt:17-19
Severity: MEDIUM
Category: code_quality

Context:

  • Pattern: Hardcoded PII constants (PREFILL_FULL_NAME, PREFILL_DOB, PREFILL_POB_ADDRESS) in production object
  • Risk: Test data embedded in production build, misleading users into thinking this data is real or pre-populated
  • Impact: UX confusion, potential compliance issues if users submit test data to real payment APIs
  • Trigger: Sample app used as production wallet or code copied to production app

Recommendation:

// Move to separate test fixture class or make conditional on BuildConfig.DEBUG:
object EthAccountDelegate {
    // For demo/testing only - DO NOT use in production
    internal val PREFILL_FULL_NAME = if (BuildConfig.DEBUG) "Test User" else null
    internal val PREFILL_DOB = if (BuildConfig.DEBUG) "1990-01-15" else null
    internal val PREFILL_POB_ADDRESS = if (BuildConfig.DEBUG) "123 Main Street, New York, NY 10001" else null

Issue 4: Integer overflow in ULong to Int coercion

ID: mappers-estimatedtxs-overflow-9b27
File: product/pay/src/main/java/com/walletconnect/pay/Mappers.kt:58
Severity: LOW
Category: code_quality

Context:

  • Pattern: etaS.coerceAtMost(Int.MAX_VALUE.toULong().toLong()).toInt() at line 58
  • Risk: Confusing logic - etaS is ULong (0 to 2^64-1), but coerced to Long then Int. Values > Int.MAX_VALUE become negative after .toInt()
  • Impact: Negative estimatedTxs displayed to users (e.g., -2147483648 txs)
  • Trigger: Backend returns etaS > 2147483647

Recommendation:

// At line 58, use proper clamping:
estimatedTxs = etaS.coerceAtMost(Int.MAX_VALUE.toULong()).toInt()

Issue 5: Base64.URL_SAFE breaks standard base64 compatibility

ID: paymentviewmodel-prefill-base64-4d11
File: sample/wallet/src/main/kotlin/com/reown/sample/wallet/ui/routes/dialog_routes/payment/PaymentViewModel.kt:176
Severity: LOW
Category: code_quality

Context:

  • Pattern: Base64.NO_WRAP or Base64.URL_SAFE used for prefill parameter encoding (line 176)
  • Risk: URL_SAFE replaces + and / with - and _. If backend expects standard base64, decoding fails
  • Impact: Prefill data not recognized by backend, user must re-enter all info
  • Trigger: Backend uses standard base64 decoder (most common)

Recommendation:

// Line 174-176: Unless backend spec requires URL-safe encoding, use standard base64:
val encoded = Base64.encodeToString(
    prefillData.toString().toByteArray(Charsets.UTF_8),
    Base64.NO_WRAP  // Remove URL_SAFE flag unless confirmed by backend API spec
)

Additional Observations

Positive:

  • Proper SSL error handling (handler.cancel() at line 1975)
  • External URL isolation (line 1939-1948 prevents navigation hijacking)
  • Structured error types with network failure mapping
  • Comprehensive unit test coverage for data models

Non-blocking:

  • Consider adding rate limiting for payment confirmation attempts
  • PaymentViewModel could benefit from coroutine exception handling wrapper
  • WebView memory leaks possible if Activity recreated during load (minor, acceptable for sample)

  • Analyze PR scope and size
  • Review WalletConnect Pay module changes
  • Review sample wallet implementation
  • Check security and architecture patterns
  • Review test coverage
  • Compile findings

@sonarqubecloud
Copy link

@jakubuid jakubuid merged commit 97131d7 into master Feb 11, 2026
22 of 26 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Feb 11, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant