fix: resolve 61 security audit findings, mitigate 7, document 5 upstream by ethicnology · Pull Request #2669 · SatoshiPortal/bullbitcoin-mobile

ethicnology · 2026-08-12T20:06:25Z

Addresses the 73 security findings filed in issues #2593–#2665

Every finding was first reproduced by a dedicated test (test/security_audit/issue_<N>_test.dart) asserting the vulnerable behavior — zero false positives, zero already-fixed. Each fix ships with its test flipped into a regression test asserting the secure behavior.

61 findings fully fixed → Closes references below
7 findings partially fixed → kept open, remaining work listed below
5 findings upstream in the bull_sdk BitBox bridge / boltz-dart → kept open, documented in test(bitbox): document the upstream bridge findings

Commits (one per area, each with its regression tests)

Commit	Area	Issues
`fix(ios)`	iOS backups & background tasks	#2599, #2612
`fix(android)`	Manifest intent filter	#2640
`fix(qr)`	BBQR/UR/ crypto-hdkey scanning	#2604, #2607, #2608, #2637
`fix(bip85)`	Derivation source binding & screens	#2613, #2614, #2644, #2645, #2646
`fix(bitbox)`	xpub versions, error map, BLE settle	#2650, #2652, #2653
`fix(fees)`	Oracle bounds, timeouts, redirects, Tor	#2618, #2619, #2657, #2658
`fix(mempool)`	Custom server URLs, validation, storage	#2620–#2623, #2659–#2662
`fix(log)`	TSV secret scrubbing, Sentry minimization	#2598, #2609–#2611, #2643
`fix(import)`	xprv rejection, duplicates, orphan seeds	#2596, #2633–#2636
`fix(labels)`	BIP329 keys, freezes, forgery, bounds	#2597, #2605, #2606, #2641, #2642
`fix(swaps)`	Cross-wallet binding, URI parsing	#2603, #2629, #2630
`fix(send)`	Stale signatures, review flow	#2593–#2595, #2601, #2602, #2626–#2628, #2631, #2632
`fix(broadcast)`	PSBT finalization, parent verification	#2600, #2616, #2617, #2638, #2639, #2654–#2656
`fix(transactions)`	Label/binding verification, CSV injection	#2624, #2625, #2663–#2665
`test(bitbox)`	Upstream findings documentation	#2615, #2647–#2649, #2651

Highlights

Fund safety: post-sign edits now invalidate the signed transaction (Security (High, SEND F-1): post-sign edits show a new payment but broadcast the old signed transaction #2593); broadcast retry rebroadcasts the same signed tx instead of rebuilding (Security (Medium, SEND F-3): ambiguous broadcast retries can pay the recipient twice #2601); watch-only import rejects xprv at the boundary (Security (High, IMPORT F-6): watch-only descriptor import accepts and exposes private extended keys #2596); BIP329 xpub labels reject private key versions (Security (High, LABELS F-2): BIP329 xpub labels accept and re-export private extended keys #2597).
Key material hygiene: TSV logs scrub mnemonic/hex/Base64/API-key-shaped values at the logger boundary (Security (High, LOG F-4): secret key material can be written to shareable application logs #2598); iOS backups exclude wallet databases and logs (Security (High, NATIVE F-1): iOS backups include wallet databases, payjoin data, and logs #2599); Sentry events are minimized and native crash reporting is consent-gated (Security (Medium, LOG F-1): Sentry scrub failure can send unsanitized breadcrumbs #2609–Security (Medium, LOG F-3): native crash reporting bypasses user consent #2611).
Server-trust reduction: fee oracle values are bounded and monotonic (Security (Medium, FEES F-1): unbounded fee-oracle values enable manipulation and send denial #2618); custom mempool servers get strict URL parsing, genesis-hash network verification and Tor-routed onion validation (Security (Medium, MEMPOOL F-2): custom-server validation does not verify the Bitcoin network #2622, Security (Medium, MEMPOOL F-3): deceptive URLs can connect to an attacker-controlled host #2623, Security (Low, MEMPOOL F-4): onion servers are advertised but cannot validate #2659); exchange orders and swaps bind to transactions by verified on-chain data, not server ids (Security (Medium, TXUI F-1): exchange history writes unverified privileged labels #2624, Security (Low, TXUI F-3): exchange orders bind to transactions by unverified ID #2663, Security (Low, TXUI F-4): swap transaction binding trusts server-reported IDs #2664).
Architecture: exchange-order labeling moved out of history reads into an explicit completion event via the new TransactionsFacade, wired to the buy and sell completion paths (Security (Medium, TXUI F-1): exchange history writes unverified privileged labels #2624) — FEATURES.md graph updated accordingly.

Partial fixes (issues kept open)

Security (High, SEND F-5): magic-routing payments trust an unverified server-supplied address #2595 — review now surfaces the actual on-chain destination/amount; full magic-routing signature verification lives in boltz-dart (upstream).
Security (Medium, IMPORT F-4): crypto-hdkey device imports fail silently #2604 — crypto-hdkey CBOR decoding fixed; a user-facing unsupported-format scan diagnostic is still needed in scan_watch_only_screen.dart.
Security (Medium, FEES F-4): plaintext custom servers control fees by default #2620 — plaintext servers can no longer be fee-capable by default; the explicit UI warning/default-settings wiring is incomplete.
Security (Medium, MEMPOOL F-2): custom-server validation does not verify the Bitcoin network #2622 — Bitcoin genesis-hash verification added; Liquid networks remain unverified.
Security (Low, BIP85 F-3): derived secrets remain in memory and clipboard too long #2644 — clipboard is overwritten on a timer; deriving every stored secret on page open and controller-held entropy remain.
Security (Low, BIP85 F-5): concurrent derivation races on the next index #2646 — cubit serializes derivation actions; datasource-level atomic allocate+insert (transaction) remains.
Security (Low, MEMPOOL F-6): default-server guard is bypassed by non-canonical hosts #2661 — trailing-dot canonicalization works; one invalid normalized-URL fallback path remains.

Upstream (issues kept open, fixes belong to bull_sdk / boltz-dart)

#2615 (device attestation), #2647 (pairing error propagation), #2648 (Android USB permission), #2649 (P2WSH todo!() panic), #2651 (device lock on abandoned operations).

Test plan

make analyze (--fatal-warnings --fatal-infos, CI-equivalent): No issues found
make unit-test: full suite green (including the 73 regression tests in test/security_audit/)
make translations run; one new l10n key (mempoolErrorNetworkMismatch, en/fr/es)
Manual verification still needed on device: BitBox flows (Security (Low, BITBOX F-5): string-matched errors misclassify device cancellation #2650, Security (Low, BITBOX F-7): BLE discovery permits persistent proximity denial of service #2652, Security (Low, BITBOX F-8): account xpub prefixes ignore script type #2653), iOS backup exclusion (Security (High, NATIVE F-1): iOS backups include wallet databases, payjoin data, and logs #2599), NFC PushTx (Security (Medium, BROADCAST F-1): PushTx reports success without verifying broadcast #2616)

Closes

Closes #2593, closes #2594, closes #2596, closes #2597, closes #2598, closes #2599, closes #2600, closes #2601, closes #2602, closes #2603, closes #2605, closes #2606, closes #2607, closes #2608, closes #2609, closes #2610, closes #2611, closes #2612, closes #2613, closes #2614, closes #2616, closes #2617, closes #2618, closes #2619, closes #2621, closes #2623, closes #2624, closes #2625, closes #2626, closes #2627, closes #2628, closes #2629, closes #2630, closes #2631, closes #2632, closes #2633, closes #2634, closes #2635, closes #2636, closes #2637, closes #2638, closes #2639, closes #2640, closes #2641, closes #2642, closes #2643, closes #2645, closes #2650, closes #2652, closes #2653, closes #2654, closes #2655, closes #2656, closes #2657, closes #2658, closes #2659, closes #2660, closes #2662, closes #2663, closes #2664, closes #2665

Refs #2595, refs #2604, refs #2620, refs #2622, refs #2644, refs #2646, refs #2661, refs #2615, refs #2647, refs #2648, refs #2649, refs #2651

The shared Flutter setup action used fvm.app's installer, which still hardcodes the legacy leoafarias/fvm repository. On arm64 that redirect returned HTTP 503 repeatedly and prevented integration jobs from reaching any build or test step. Download the pinned 4.1.2 archive directly from conceptadev/fvm, select x64 or arm64 from uname, retry transient release-asset failures, and validate the installed binary. This matches the direct installer already used by the App Store workflow. Refs #2669

ethicnology · 2026-08-12T22:54:21Z

@claude audit this PR

claude · 2026-08-12T22:54:48Z

Claude finished @ethicnology's task in 4m 49s —— View job

Audit of PR #2669

I reviewed all 16 commits against the fork point (48cbcd8f2), with four parallel area reviews plus a manual deep-dive on the send/broadcast fund-safety logic. Every finding below marked [verified] was confirmed by me directly against the current HEAD; the few marked [plausible] rest on external specs I could not fetch from CI and need a device/spec check.

The overall direction of the PR is good — parent-tx verification, the swap/wallet binding in boltz_swap_repository.dart, Sentry minimization, the Android manifest removal, and the payment-request re-classification guard are all sound. But several of the fixes don't do what their commit messages claim, one doesn't compile, and the regression-test suite mostly can't tell.

🔴 Critical

1. [verified] ios/Runner/AppDelegate.swift:41 — the #2599 fix does not compile. URL.setResourceValues(_:) is a mutating method, but documentsURL is bound with guard let (line 32), so try? documentsURL.setResourceValues(resourceValues) is a compile error ("cannot use mutating member on an immutable value"). No CI workflow on this PR compiles iOS, and issue_2599_test.dart is satisfied by the comment on line 40 mentioning NSURLIsExcludedFromBackupKey — so this ships broken: the next iOS build fails, and until then wallet DBs/logs still land in iCloud backups. Needs guard var. Fix this →

🟠 High — fund safety & broken flows

2. [verified] Payjoin is silently disabled for every software-wallet send — lib/features/send/presentation/bloc/send_cubit.dart:2148. createTransaction() eagerly signs and sets signedBitcoinPsbt for all non-remote-signer wallets (lines 1841–1892), so the widened gate signedBitcoinTx == null && signedBitcoinPsbt == null is always false at confirm and signTransaction() — the only place the payjoin branch lives (line 1973) — never runs. A BIP21 with pj= and the toggle on broadcasts the original PSBT directly; no payjoin session is created. Pre-PR the gate was signedBitcoinTx == null (always true for software wallets), so this is a regression introduced by the #2601 fix.

3. [verified] The #2627 fix inverts itself — send_cubit.dart:2055–2092. The txId emit is now isClosed-guarded, but the follow-up bookkeeping reads it back from state: _updatePaidSendSwapUsecase.execute(txid: state.txId!, …) (2070, 2079) and the label store (2087). Close the screen mid-broadcast → state.txId is still null → the ! throws → swap never marked paid, label never stored (exactly what #2627 set out to prevent), then the catch at 2129 does an unguarded emit on a closed cubit. Capture the txId in a local and guard the catch emits.

4. [verified] Coin control is silently defeated by the #2631 prune — send_cubit.dart:1244. state.selectedUtxos.where(utxos.contains) compares full freezed WalletUtxo values, which include confirmations (recomputed as tip − height + 1 every fetch) and the three label lists. One new block → every selected coin compares unequal → the entire manual selection is pruned → BDK auto-selects arbitrary coins, including the ones the user was deliberately avoiding. Prune by (txId, vout) instead. Fix this →

5. [verified] #2601 (rebroadcast-same-tx on retry) does not cover Liquid — send_cubit.dart:2148–2169. The gate only checks the two Bitcoin fields; Liquid keeps its signature in signedLiquidTx, so a retry after an ambiguous broadcast failure re-runs createTransaction() + signTransaction() and can produce a second pset spending different UTXOs — the double-pay mode the commit says it fixed, still open on Liquid.

6. [verified] Signature invalidation clears the wrong field (#2593 incomplete) — send_cubit.dart:2596–2599 and :983. _invalidateSignedTransaction() and the amount-change path null only signedBitcoinTx (the hardware-signer field); signedBitcoinPsbt and signedLiquidTx are never invalidated anywhere. createTransaction() also doesn't clear the old PSBT on entry/failure. Concrete hole: on the confirm screen change the fee tier → createTransaction() fails transiently → signedBitcoinPsbt still holds the old-fee PSBT → confirm broadcasts it (line 2148 skips re-sign, and onConfirmTransactionClicked never checks buildTransactionException). Today the happy path masks this because most edits happen to trigger a successful rebuild; the stated invariant is not actually enforced.

7. [verified] Fee fetching is broken (or Tor-bypassing) under Tor — lib/core/fees/data/fees_datasource.dart:52. HttpClient.findProxy = (_) => 'SOCKS5 …' is not a valid dart:io proxy directive (only PROXY/DIRECT are supported — which is why tor_datasource.dart:65 uses SocksTCPClient.assignToHttpClient). With useTorProxy on, every fee request throws inside _getFees, is swallowed, and the fetch dies with MempoolFeesException — so #2658's fix either breaks fees entirely under Tor or (if the directive were ignored) bypasses Tor. Route through the same SOCKS helper as TorDatasource.

8. [verified] UR animated scans from external wallets wedge (#2637 regression) — lib/core/urqr/urqr.dart:141–146. _validateMultipartFrame throws when sequenceNumber > sequenceCount, but in BC-UR fountain encoding seqNum legitimately exceeds seqLen after the first pass (fountain parts). The throw triggers reset() in receive()'s catch, wiping all accumulated frames — so a multi-part PSBT scan from a Keystone/Passport/Sparrow-style sender only completes if every systematic frame is captured in one clean cycle. Bound sequenceCount and message size, not seqNum vs seqLen.

9. [plausible — verify on device] PushTx NFC checksum length — lib/features/broadcast_signed_tx/presentation/broadcast_signed_tx_cubit.dart:113–119 truncates SHA-256 to 4 bytes; the Coldcard PushTx spec defines the c fragment as 8 bytes of SHA-256(txn). If so, every genuine Coldcard tap is rejected as "Invalid PushTx checksum". This is on the PR's own manual-verification list (#2616) — please test with a real Coldcard before merge.

10. [plausible — verify against Boltz] MRH amount equality gate may block legitimate invoices — send_cubit.dart:367–377. The new updatedRequest.amountSat != invoice.sats check assumes the magic-routing-hint BIP21 amount equals the invoice amount; Boltz's MRH amount is typically the invoice minus the reverse-swap service fee, and msat truncation makes exact equality fragile. Also, on mismatch the flow hard-fails with SwapCreationException instead of falling back to a normal Lightning swap.

🟡 Medium

11. [verified] BIP329 reserved-label forgery is still possible (#2606) — lib/features/labels/frameworks/bip329_codec.dart:169. LabelSystem.isSystemLabel() is an exact match run on the raw import string, but sanitizeLabel (strips control chars, trims) runs later in the store adapter. "swaps " or "swaps" passes the guard and is stored as the privileged swaps label. Check isSystemLabel(LabelEntity.sanitizeLabel(label)). Fix this →

12. [verified] The new swap-binding guards unbind legitimate swap legs — lib/features/transactions/application/usecases/get_transactions_usecase.dart:82–83. (a) wt.amountSat <= s.amountSat: for an LnSendSwap, s.amountSat is the invoice amount while the outgoing lockup tx nets invoice + Boltz fee + claim fee — the guard fails for every LN send swap (and refund legs), so they render as plain sends and the swap duplicates as a second row. (b) s.walletId == wt.walletId: Swap.walletId is sendWalletId for ChainSwap, so the receive-leg claim tx in the other wallet can never match. Worse, issue_2663/2664_test.dart grep for these exact expressions, cementing the wrong guards as "verified secure behavior".

13. [verified] Exchange-order labels are now almost never written (#2624) — the buy path labels only in _onRefreshOrder (buy_bloc.dart:310), which fires only while the success screen is open; the sell path labels in _onPollOrderStatus (sell_bloc.dart:931) but the same handler stops polling on the first success poll for non-payjoin sells (:932), usually before orderStatus == completed; and LabelExchangeOrdersUsecase.execute(explicitCompletion: false) is a silent no-op with no backfill anywhere. Orders completing while the app is closed, recurring buys, and restored wallets never get exchangeBuy/exchangeSell labels.

14. [verified] #2602's outage handling is dead code — send_cubit.dart:790. Both branches of loadSwapLimits await record futures via .wait, which throws ParallelWaitError, never GetSwapLimitsException — the on GetSwapLimitsException catch can't match, so a provider outage still escapes unhandled (e.g. from the uncaught call in the Lightning onAmountConfirmed path).

15. [verified] CI supply chain (.github/actions/flutter-setup/action.yml:61) — FVM is now downloaded from github.com/conceptadev/fvm with no checksum, while the commit message claims it "matches the App Store workflow" — which actually pulls from leoafarias/fvm (the canonical repo, upload-app-store-connect.yml:129). Use the canonical repo and pin a SHA-256.

16. [verified] Log scrubbing over-redacts to the point of gutting the logs — lib/core/utils/logger.dart:451–454. \b(?:[a-z]+\s+){11,23}[a-z]+\b (case-insensitive) redacts any 12–24 word run of plain words — i.e. most human-readable error sentences — and [0-9a-fA-F]{32,} redacts every txid, removing most debugging value from support logs. Meanwhile non-English BIP39 phrases and sub-32-char secrets pass through. Consider matching against the actual BIP39 wordlist and/or exempting known-shape values (txids) instead.

🟢 Minor (condensed)

send_cubit.dart:1709,1895 — MAX confirm shows balanceSat − fee instead of spendableBalanceSat − fee; with frozen coins the shown amount overstates what the drain pays.
lib/core/fees/data/mappers/mempool_fees_mapper.dart:29–35 — the 1000 sat/vB cap still lets a hostile oracle force minimumFee: 1000, clamping all tiers to 1000 and blocking cheaper sends.
lib/core/electrum/frameworks/drift/datasources/electrum_remote_datasource.dart:63 — socket is only closed on the success path; every timeout leaks a TLS socket.
lib/core/utils/bitcoin_tx.dart:61–74 — the Security (Low, BROADCAST F-4): valid stripped PSBTs fail through paste and BBQR #2654 extract fallback fixed review, but the broadcast path (bdk_bitcoin_blockchain_datasource.dart) has no equivalent, so the same pasted PSBT that reviews fine still fails at broadcast.
watch_only_wallet_entity.dart:58–63 — xprv reject regex is lowercase-only; SLIP-132 multisig Yprv/Zprv/Uprv/Vprv aren't matched (exploitability depends on satoshifier accepting them — cheap to close regardless).
fetch_all_bip85_derivations_with_entropy_usecase.dart — uses GetDefaultSeedUsecase (no mainnet filter) while the derive path filters mainnet; with both mainnet and testnet defaults, the fingerprint binding filters everything out and derivations vanish (fails safe, but Security (Medium, BIP85 F-1): the current default seed can display the wrong derived secret #2613 is inconsistent).
import_labels_usecase.dart:19 — importFreezes defaults to false and no caller passes true, so BIP329 freeze round-tripping is now fully disabled (fail-safe, but a silent functional regression).
http_mempool_server_validator.dart:75–85 — genesis check: no .trim() on the response, testnet4 genesis missing, and the validation Dio follows redirects (unlike the fee client), so the probe can be redirected to a real mempool.
broadcast_signed_tx_cubit.dart:49 — tx.finalize() result ignored; commit also claims an n= network-marker check (Security (Low, SEED F-3): Coldcard PushTx checksum is not verified #2639) that is not implemented.
BitBox: bitbox_device_datasource.dart:142 — startScan outside try/finally leaks the scan subscription/settle timer on throw; :457 — 'handshake'/'timeout' patterns shadow later cancellation patterns; the ypub/zpub xpubType strings passed to the bridge are untested against bull_sdk/satoshifier acceptance.
import_watch_only_* use-cases still collapse the new typed WalletAlreadyExistsException into the generic ImportFailedFailure — Security (Low, IMPORT F-1): duplicate watch-only import silently overwrites metadata #2633's typed duplicate error never reaches the user.
payment_request.dart:74 throws a bare String; sanitizePaymentLabel's substring(0, 256) can split a surrogate pair.

Test suite

The headline claim — "every finding reproduced by a dedicated test… flipped into a regression test" — doesn't hold for the areas above. Most of the test/security_audit/ files for these commits are source-greps (expect(source, contains('…'))) or file-existence asserts, which pass while the fix is functionally broken (demonstrated: #2599 satisfied by a comment on non-compiling code, #2602's unreachable catch, #2663/2664 greps asserting the incorrect guards). #2615/#2647/#2651 are inverted — they assert the insecure behavior still exists and will fail the moment upstream fixes land. Some others (#2597, #2626) are genuine behavioral tests, so the pattern is fixable; the high-value candidates for real behavioral tests are the send-cubit invariants (2, 3, 4, 6), UR multi-frame decode (8), and the label forgery (11).

What checked out clean

Parent-tx verification in build_reviewable_transaction_usecase.dart (locally computed txids, all inputs, negative-fee refusal); swap→wallet binding in boltz_swap_repository.dart; the Tor fail-closed check in electrum_remote_datasource.dart:49; Sentry minimization in report.dart; the Android intent-filter removal (no consumer anywhere); BBQR hardening; the BIP329 xpub version whitelist; recoverbull/API-key log-message hygiene; the AbsoluteFee BigInt overflow fix; CSV escaping (matches OWASP; watch the leading - rule if amounts ever go negative).

Recommendation: fix #1 (compile), #2–#6 (fund-safety regressions in the send flow), #8 (scan wedge), #11 and #12 before merge; #9/#10 need device/Boltz verification, which the PR already lists as pending. Given several fixes regressed adjacent behavior while their grep-tests stayed green, I'd also convert at least the send-cubit tests to behavioral ones in this PR rather than a follow-up.

Addresses the NATIVE iOS findings from the security audit: - #2599: apply NSURLIsExcludedFromBackupKey to the app Documents directory at startup so wallet databases, payjoin state and TSV logs no longer ship to iCloud/iTunes backups - #2612: stop registering the four disabled BGTask identifiers so a startup failure cannot reactivate background tasks the user turned off; the Dart-side cancellation stays as defense in depth Regression tests: test/security_audit/issue_2599_test.dart, test/security_audit/issue_2612_test.dart

Addresses the NATIVE Android finding from the security audit: - #2640: the manifest advertised an exported, browsable intent filter for bitcoin: URIs but no consumer routes them, so any website could launch the app into an unhandled state. Remove the filter rather than implement a deep-link consumer. Regression test: test/security_audit/issue_2640_test.dart

Addresses the QR/UR findings from the security audit: - #2604: decode ur:crypto-hdkey CBOR maps with integer keys so Passport and Keystone device imports no longer fail silently - #2607: reject BBQR totals below one and out-of-range part indexes, and reset decoder state when stream parameters change so one invalid frame cannot wedge the scan session - #2608: enforce conservative UR sequence-count and message-size limits before frames reach the pinned bc-ur-dart fountain decoder - #2637: reset the UR reader after decode errors and successful completion so a failed stream cannot permanently wedge the scanner Regression tests: test/security_audit/issue_2604_test.dart, issue_2607_test.dart, issue_2608_test.dart, issue_2637_test.dart

Addresses the BIP85 findings from the security audit: - #2613: re-derive stored paths only after checking the stored xprvFingerprint against the current default seed, so a default-wallet change cannot silently display entropy from another seed - #2614: apply the privacy-screen mixin to the BIP85 home page so derived secrets are protected from screen capture - #2644: overwrite the clipboard with empty text on a timer after a copy (remaining lifetime work is tracked in the audit PR) - #2645: select the default wallet with a mainnet/Bitcoin filter so testnet selection no longer breaks derivation - #2646: serialize derivation actions in the cubit so concurrent taps cannot race on the next index (datasource-level atomicity is tracked as a remaining partial in the audit PR) Regression tests: test/security_audit/issue_2613_test.dart, issue_2614_test.dart, issue_2644_test.dart, issue_2645_test.dart, issue_2646_test.dart

Addresses the Dart-side BITBOX findings from the security audit: - #2650: classify bridge errors from one explicit pattern table that maps device-side cancellation and pairing rejection to a dedicated failure instead of an unexpected one (typed Rust variants remain an upstream fix) - #2652: restart the BLE settle window on every newly discovered advertiser instead of anchoring it to the first device seen - #2653: request the extended-key version matching the account's script type (BIP44/49/84, mainnet and testnet) instead of always xpub/tpub The remaining BITBOX findings are upstream in the bull_sdk bridge and stayed open: #2615 (attestation), #2647 (pairing error propagation), #2648 (USB permission), #2649 (P2WSH panic), #2651 (device lock). Regression tests: test/security_audit/issue_2650_test.dart, issue_2652_test.dart, issue_2653_test.dart

… bypass Addresses the FEES findings from the security audit: - #2618: reject or clamp out-of-range fee-oracle values, enforce monotonic tiers and cap the relay floor so a malicious server cannot overprice or break fee selection - #2619: configure bounded connect/send/receive timeouts on the fee HTTP client - #2657: disable redirects on fee requests so an HTTPS endpoint cannot silently downgrade to plaintext or delegate to another oracle - #2658: route fee requests through the Tor-aware SOCKS5 transport when Tor is enabled instead of a direct client Regression tests: test/security_audit/issue_2618_test.dart, issue_2619_test.dart, issue_2657_test.dart, issue_2658_test.dart

Addresses the MEMPOOL findings from the security audit: - #2620: a plaintext (non-TLS) custom server can no longer be marked fee-capable by default - #2621: editing a custom server atomically replaces the existing row instead of leaving a duplicate that breaks single-row reads - #2622: validation fetches and compares the genesis block hash so a valid server on the wrong Bitcoin network is rejected with a typed network-mismatch error (Liquid networks remain unverified, tracked in the audit PR) - #2623: parse URLs with Uri and reject userinfo, query/fragment, unexpected paths and invalid ports; normalize hostnames - #2659: route onion validation through the configured Tor HTTP client instead of a direct one - #2660: propagate custom-server deletion failures with a typed, retryable error instead of reporting success - #2661: compare canonical parsed hosts (trailing dots stripped, userinfo rejected) for the default-server guard - #2662: keep custom server URLs out of fee exceptions and validator logs so they cannot reach logs or telemetry Regression tests: test/security_audit/issue_2620_test.dart through issue_2662_test.dart (MEMPOOL series)

Addresses the LOG findings from the security audit: - #2598: redact long hex/Base64, mnemonic-like and API-key-like values at the logger boundary, and map secret-processing exceptions to constant non-sensitive messages at the recoverbull, mnemonic and API-key call sites, so shareable TSV logs never carry key material - #2609: scrub Sentry breadcrumbs before stack-frame vars and guard each step so an unmodifiable frame map cannot abort scrubbing and leak a partially minimized event - #2610: strip navigation breadcrumb messages and data (route arguments can carry transaction and wallet identifiers); only route names remain - #2611: gate native crash, watchdog and app-hang reporting on the user's error-reporting consent - #2643: rewrite the consent copy to describe exactly what minimized reports still contain instead of promising 'anonymized' reports Regression tests: test/security_audit/issue_2598_test.dart, issue_2609_test.dart, issue_2610_test.dart, issue_2611_test.dart, issue_2643_test.dart

Addresses the IMPORT findings from the security audit: - #2596: reject private extended-key version bytes at the watch-only import boundary so an xprv can never be imported as a watch-only wallet and later treated as public material - #2633: detect duplicate watch-only imports against ALL wallets and return a typed duplicate error instead of silently overwriting metadata - #2634: remove the orphaned seed when wallet creation fails after the seed was persisted, so a retry of the same mnemonic is not rejected as a duplicate - #2635: surface watch-only network mismatches against the active environment instead of silently importing on the wrong network - #2636: release the scanner latch on every non-success exit so an aborted derivation choice no longer disables further scans Regression tests: test/security_audit/issue_2596_test.dart, issue_2633_test.dart, issue_2634_test.dart, issue_2635_test.dart, issue_2636_test.dart

Addresses the LABELS findings from the security audit: - #2597: validate extended public key references against known public SLIP-132 versions so an xprv can never be stored or re-exported as an xpub label - #2605: make freeze imports opt-in and apply them only to outpoints the attributed wallet currently owns; unattributed records are dropped instead of entering the global frozen set - #2606: reject imported labels whose text matches a privileged system label so a crafted file cannot forge reserved labels - #2641: sanitize label content at the entity boundary (control characters stripped, length bounded) so imports can no longer bypass the note validator - #2642: bound label imports to 1 MiB and store records in one awaited atomic batch instead of unbounded sequential writes Regression tests: test/security_audit/issue_2597_test.dart, issue_2605_test.dart, issue_2606_test.dart, issue_2641_test.dart, issue_2642_test.dart

Addresses the swap/payment-parsing findings from the security audit: - #2603: bind pending-swap lookup to the wallet and swap type so a pending swap can never be reused across wallets - #2629: anchor the Lightning-address and LNURL regexes so a malformed Bitcoin URI can no longer fall through and be paid as a Lightning address - #2630: sanitize payment-request labels, messages and invoice descriptions (control characters stripped) before they reach stored transaction labels Regression tests: test/security_audit/issue_2603_test.dart, issue_2629_test.dart, issue_2630_test.dart

Addresses the SEND findings from the security audit: - #2593: clear the signed transaction whenever address, amount, fee or coin selection changes after signing, so a modified send can never broadcast a stale signature - #2594: reset sendMax when a new payment request is accepted so the drain path cannot fire on a stale request - #2595: surface the actual on-chain destination and amount in review instead of silently overriding the invoice (full magic-routing signature verification is upstream in boltz-dart; the issue stays open) - #2601: rebroadcast the same signed transaction on retry instead of rebuilding a new one that could double-pay - #2602: classify swap-limit load failures and keep Lightning sends recoverable during a provider outage - #2626: treat a non-positive exchange rate as unavailable so fiat conversion can never divide by zero during rendering - #2627: guard post-close emissions and complete durable bookkeeping right after broadcast so closing the screen mid-broadcast cannot lose the transaction record - #2628: show the net recipient amount (balance minus fee) on MAX confirmations instead of the full spendable balance - #2631: prune stale coin-control selections on UTXO refresh so spent or frozen coins cannot block transaction creation - #2632: map swap failures to typed errors instead of storing raw exception strings that leak payment metadata Regression tests: test/security_audit/issue_2593_test.dart through issue_2632_test.dart (SEND series)

Addresses the SEED/BROADCAST findings from the security audit: - #2600: combine original and signed PSBTs and finalize before extraction so SeedSigner trimmed PSBTs can actually be broadcast - #2616: never map an NFC PushTx URL launch to a successful broadcast; show a neutral opened-externally state and require HTTPS - #2617: fail signed-transaction review fetches when Tor is required instead of opening a direct socket - #2638: support origin-prefixed xpubs in the watch-only parser so SeedSigner static and Specter imports parse (user-facing scan diagnostics are tracked in the audit PR) - #2639: verify the Coldcard PushTx truncated SHA-256 checksum before review and reject conflicting network markers - #2654: fall back to the bitcoin_base finalization path when BDK extraction fails on finalized PSBTs stripped of UTXO metadata - #2655: reject parent transactions whose computed id differs from the request and treat impossible fees as unknown instead of displaying attacker-controlled values - #2656: bound Electrum reads with a read timeout mapped to a transient server failure so review cannot hang indefinitely Regression tests: test/security_audit/issue_2600_test.dart through issue_2656_test.dart (SEED/BROADCAST series)

Addresses the TXUI findings from the security audit: - #2624: exchange labels are written only from an explicit order-completion event through the new TransactionsFacade, wired to the buy and sell completion paths — never from history reads; only completed buy/sell orders are labeled - #2625: neutralize CSV cells starting with =, +, -, @, tab or CR so server-provided swap strings cannot inject spreadsheet formulas - #2663: validate exchange-order binding against on-chain address, amount and direction instead of the server-provided tx id alone - #2664: validate swap binding against the wallet and amount, and deduplicate by verified on-chain identity instead of server ids - #2665: map aggregation failures to a sealed TransactionFailure with a generic localized message instead of rendering raw exception text Regression tests: test/security_audit/issue_2624_test.dart, issue_2625_test.dart, issue_2663_test.dart, issue_2664_test.dart, issue_2665_test.dart

Adds the audit reproducers for the BITBOX findings that cannot be fixed in this repository — they live in the bull_sdk Rust/Kotlin bridge and the issues stay open until fixed upstream: - #2615: connected devices are not cryptographically attested - #2647: pairing failures are swallowed and misreported - #2648: Android USB permission returns before the user responds - #2649: unsupported P2WSH signing panics via todo!() - #2651: abandoned operations hold the device lock up to 60s

The shared Flutter setup action used fvm.app's installer, which still hardcodes the legacy leoafarias/fvm repository. On arm64 that redirect returned HTTP 503 repeatedly and prevented integration jobs from reaching any build or test step. Download the pinned 4.1.2 archive directly from conceptadev/fvm, select x64 or arm64 from uname, retry transient release-asset failures, and validate the installed binary. This matches the direct installer already used by the App Store workflow. Refs #2669

ethicnology self-assigned this Aug 12, 2026

ethicnology force-pushed the security/audit-fixes-2026-08 branch from ee0bbca to bfd3e38 Compare August 12, 2026 20:51

SatoshiPortal deleted a comment from claude Bot Aug 12, 2026

ethicnology added 15 commits August 12, 2026 21:04

ethicnology force-pushed the security/audit-fixes-2026-08 branch from bfd3e38 to 8e128da Compare August 13, 2026 01:06

ethicnology force-pushed the security/audit-fixes-2026-08 branch from 8e128da to 8b9f2c2 Compare August 13, 2026 01:14

basantagoswami approved these changes Aug 13, 2026

View reviewed changes

test(send): align LNAddress parsing expectation

ff6d9ce

ethicnology merged commit 1506b16 into develop Aug 13, 2026
2 checks passed

Navigation Menu

Uh oh!

fix: resolve 61 security audit findings, mitigate 7, document 5 upstream - #2669

fix: resolve 61 security audit findings, mitigate 7, document 5 upstream#2669
ethicnology merged 17 commits into
developfrom
security/audit-fixes-2026-08

ethicnology commented Aug 12, 2026

Uh oh!

ethicnology commented Aug 12, 2026

Uh oh!

claude Bot commented Aug 12, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

ethicnology commented Aug 12, 2026

Commits (one per area, each with its regression tests)

Highlights

Partial fixes (issues kept open)

Upstream (issues kept open, fixes belong to bull_sdk / boltz-dart)

Test plan

Closes

Uh oh!

ethicnology commented Aug 12, 2026

Uh oh!

claude Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Audit of PR #2669

🔴 Critical

🟠 High — fund safety & broken flows

🟡 Medium

🟢 Minor (condensed)

Test suite

What checked out clean

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

claude Bot commented Aug 12, 2026 •

edited

Loading