"Please help me review the PayNym / BIP47 integration in this BlueWallet app repo ( https://github.com/linkinparkrulz/BlueWallet/releases/tag/v1.0.0 ). I am wondering about cryptographic and security best practices and if you can see any obvious gaps at first blush. e.g. How are secrets and credentials generated, stored, shared, refreshed and/or deleted, for example?"
"Please double check the high and medium priority issues you brought up, confirm their validity and offer suggestions for remediation or improvement, and write each of these out in paragraph format as much as possible. Note filenames, function names and/or line numbers where relevant in your review."
Below are the confirmed HIGH and MEDIUM priority findings, each double-checked against the actual source code with file names, function names, and line numbers.
File: blue_modules/paynym/PaynymDirectory.ts
Functions: create() (around the [CREATE DEBUG] block) and claim() (around the [CLAIM DEBUG] block)
This finding is confirmed. The create() method logs the payment code being registered (code.substring(0, 20)), and the claim() method logs both the API token (token.substring(0, 20)) and the cryptographic signature (signature.substring(0, 20)). Both methods also call console.trace(), which dumps the full call stack. While the values are truncated to 20 characters, this is still a meaningful amount of data — for base64-encoded tokens and signatures, 20 characters represents roughly 15 bytes of the actual value, which is enough to significantly narrow a brute-force search space. More importantly, console.trace() reveals internal application structure and execution paths. On Android, these logs are written to logcat and can be read by any app with READ_LOGS permission (on older Android versions) or by anyone with USB debugging access. On iOS, they appear in the device console accessible via Xcode.
Remediation: Remove all [CREATE DEBUG] and [CLAIM DEBUG] log blocks entirely, or gate them behind a compile-time __DEV__ flag (e.g., if (__DEV__) { console.log(...) }). React Native's __DEV__ is false in production builds, so this would prevent the logs from appearing in release builds while preserving them for development. The console.trace() calls should be removed unconditionally — stack traces should never appear in production logs.
File: class/wallets/abstract-hd-electrum-wallet.ts
Function: getBIP47NotificationTransaction() at line 1740
This finding is confirmed. The function iterates through all wallet transactions and checks whether any transaction output's scriptPubKey.addresses includes the receiver's notification address. If it finds such a transaction, it returns it immediately as the notification transaction. The code comment on line 1749 explicitly states: "not gona verify it here, will just trust it." This means any payment to someone's notification address — even an accidental one, or a deliberate non-notification payment — will be treated as a valid BIP47 notification. The consequence is that the wallet may believe it has already notified a counterparty when it hasn't, which means the counterparty won't be able to derive the shared addresses and any funds sent to those addresses will be unspendable by the recipient.
Remediation: After finding a candidate transaction, the function should verify that the transaction contains an OP_RETURN output with exactly 80 bytes of data (the blinded payment code). Ideally, it should also attempt to unblind the OP_RETURN using the first input's outpoint and verify that the unblinded data is a valid BIP47 payment code matching the sender's. At minimum, checking for the presence and correct length of an OP_RETURN output would catch the most common false-positive case (a plain payment to the notification address). The @spsina/bip47 library likely provides methods to validate notification payloads that could be leveraged here.
File: class/wallets/abstract-hd-electrum-wallet.ts
Function: addBIP47Receiver() at line 2068
This finding is confirmed but requires nuance. The function simply checks for duplicates and then pushes the payment code onto _send_payment_codes. It does not verify that a notification transaction has been broadcast and confirmed on-chain. However, this is a low-level data structure method, and the actual workflow that calls it (the UI layer) may enforce the correct ordering. The risk is that if any code path calls addBIP47Receiver() before the notification transaction is confirmed, the wallet will begin generating derived addresses that the counterparty cannot compute (because they haven't received the notification). Funds sent to those addresses would be effectively lost to the recipient.
Remediation: Add a precondition check inside addBIP47Receiver() that calls getBIP47NotificationTransaction(paymentCode) and throws an error (or returns without adding) if no notification transaction is found. This creates a defense-in-depth guarantee that the data model cannot enter an inconsistent state regardless of which code path invokes it. Alternatively, if there are legitimate reasons to add a receiver before notification (e.g., optimistic UI), document this clearly and add a verified: boolean flag to the payment code entry so downstream code can distinguish between notified and un-notified counterparties.
File: blue_modules/paynym/PaynymDirectory.ts, lines 10, 15, 638, 655, 706, 712
Functions: getPaynymInfoCached(), cachePaynymInfo(), clearCache()
This finding is confirmed. The STORAGE_KEY_PREFIX is 'paynym_dir_' (line 15), and PayNym info objects (containing nymName, nymID, segwit flag, and follower/following counts) are serialized to JSON and stored via AsyncStorage.setItem() (line 655). AsyncStorage on Android is backed by an unencrypted SQLite database, and on iOS by unencrypted plist files. While the cached data is technically public information (anyone can query the PayNym API for any payment code), the set of payment codes the user has looked up constitutes private metadata. An attacker with physical device access or a backup extraction could determine which payment codes the user has interacted with or searched for, which undermines the privacy goals of BIP47.
Remediation: Either encrypt the cached data before storing it (using the app's existing encryption module at blue_modules/encryption.ts), or use a secure storage backend like react-native-keychain / react-native-sensitive-info for this data. A simpler alternative would be to use only in-memory caching (a plain JS Map) and accept that PayNym lookups will require network requests after app restart. Given that this is a cache of public data, the in-memory-only approach is probably the best tradeoff between privacy and complexity.
File: blue_modules/paynym/PaynymDirectory.ts, line 13 and the makeRequest() method
Endpoint: https://paynym.rs/api/v1
This finding is confirmed. All API calls use the standard fetch() API with no additional TLS verification. The makeRequest() method constructs a standard fetch() call with JSON headers and a POST body. There is no certificate pinning, public key pinning, or custom TLS configuration. This means the connection is vulnerable to man-in-the-middle attacks by anyone who can install a trusted CA certificate on the device (e.g., corporate MDM, compromised CA, or a state-level adversary). A MITM attacker could intercept the /api/v1/nym response and return a different payment code for a given nymName, causing the user to send funds to the attacker's address.
Remediation: Implement certificate pinning for the paynym.rs domain. In React Native, this can be done with libraries like react-native-ssl-pinning or by configuring the native networking layer. Pin the leaf certificate's public key (SPKI hash) rather than the full certificate, so that certificate rotation doesn't break the app. Additionally, consider adding a fallback verification step: after resolving a payment code via the API, the UI could display the full payment code and prompt the user to verify it out-of-band before sending funds. This is especially important for first-time connections.
File: class/wallets/abstract-hd-electrum-wallet.ts, line 2136
Function: _getBIP47AddressReceive() and the _address_to_wif_cache object
This finding is confirmed. When _getBIP47AddressReceive() derives a BIP47 joint address at line 2136, it stores the WIF-encoded private key in this._address_to_wif_cache[address] = hdNode.toWIF(). This cache is a plain JavaScript object that persists for the lifetime of the wallet instance. There is no eviction policy, no maximum size, and no mechanism to zero the memory when the wallet is locked or the app is backgrounded. JavaScript does not provide reliable memory zeroing (strings are immutable and garbage collection timing is non-deterministic), but the lack of any eviction means private keys accumulate indefinitely. On a long-running session with many BIP47 counterparties, this cache could contain dozens or hundreds of private keys.
Remediation: While JavaScript's memory model makes true secure erasure impossible, there are still meaningful mitigations. First, consider using a Map with a bounded size (LRU cache) so that old keys are at least dereferenced and eligible for garbage collection. Second, clear the cache when the app enters the background (via React Native's AppState listener) — this won't guarantee immediate memory zeroing, but it reduces the window of exposure. Third, consider whether the cache is necessary at all: if WIF keys are only needed at transaction signing time, they could be derived on-demand from the HD node rather than cached. The getBIP47FromSeed() method already caches the BIP47 instance, so re-deriving individual keys from it should be fast.
File: blue_modules/paynym/PaynymDirectory.ts, line 13
Constant: PAYNYM_API_BASE = 'https://paynym.rs/api/v1'
This finding is confirmed. Every API call in the PaynymDirectory class goes to this single hardcoded endpoint. There is no fallback URL, no configurable endpoint, and no mechanism for the user to specify an alternative server. If paynym.rs goes offline, is blocked by a network provider, or is compromised, all PayNym functionality in the app ceases to work. The rate-limiting retry logic (single retry with a configurable wait) only handles transient failures, not sustained outages. From a security perspective, a single centralized dependency also creates a single point of trust: the server operator can correlate IP addresses with payment code lookups, building a metadata graph of who is transacting with whom.
Remediation: Make the API base URL configurable, either through a settings screen or an environment variable. This allows power users to run their own PayNym directory server (the paynym.rs API specification is documented in paynym-api.md in this repo). Additionally, consider implementing a simple failover mechanism: maintain a list of known directory servers and try the next one if the primary fails. For the privacy concern, consider routing API requests through Tor (React Native Tor libraries exist, e.g., react-native-tor) or at minimum documenting the privacy implications of the centralized lookup so users can make informed decisions. Human Note: The Ashigaru PayNym directory may be accessible through a Tor Onion URL such as http://paynym25[...]ivad.onion (Confirm this independently.)
File: class/wallets/hd-segwit-bech32-wallet.ts, lines 151 and 154
Function: generatePaynymClaimSignature()
This finding is confirmed. Inside the generatePaynymClaimSignature() method, the code uses const ecpairModule = require('ecpair') (line 151) and const ecc = require('../../blue_modules/noble_ecc') (line 154) instead of top-level ES module imports. The @ts-ignore comments on lines 149 and 153 acknowledge that this is a workaround. Dynamic require() inside a method body means that module resolution errors won't surface until the method is actually called at runtime, rather than at app startup or build time. If the ecpair package is missing or the path to noble_ecc changes, the user will encounter a crash at the exact moment they try to claim their PayNym — a critical user-facing operation. Additionally, the code on lines 156-159 tries multiple export patterns (ecpairModule.ECPairFactory || ecpairModule.default || ecpairModule) to handle different module formats, which suggests fragility in the dependency resolution.
Remediation: Move these require() calls to top-level import statements at the top of the file, consistent with how the rest of the codebase handles dependencies. The abstract-hd-electrum-wallet.ts parent class already imports ecc from noble_ecc at the top level, so the child class should be able to do the same. If there's a specific reason for the dynamic import (e.g., circular dependency or conditional loading), document it clearly and add a try/catch with a meaningful error message so that failures are diagnosable.