bitchat-android/docs/security-review-jul-27.md
callebtc bc49c71ea0
security: stop logging Noise key material, reduce noisy logging app-wide (#775)
C1 from security review: SymmetricState/HandshakeState logged raw X25519
shared secrets, chaining keys, and handshake hashes in hex to logcat on
every handshake, in release builds. A logcat transcript of a handshake
allowed full session decryption. Both classes no longer log at all.

Also reduces excessive logging across the app (~50% fewer log calls in
the noisiest files):

- NoiseSession emits one line per completed handshake; per-message
  encrypt/decrypt and per-handshake-step debug logs removed
- Removes all content/key logging: decrypted DM content, file names,
  payload hex dumps, pubkeys, event IDs, lat/lon, peer IPs, arti log
  forwarding
- Collapses multi-line banner/emoji log sequences into single factual
  lifecycle lines (connect/disconnect, relay/Tor state transitions)
- Keeps security-relevant warnings (signature failures, replay
  detection, key mismatches, panic wipe) in compact form

No logic changes. Includes the full security review report in
docs/security-review-jul-27.md.
2026-07-27 10:33:29 +02:00

17 KiB
Raw Permalink Blame History

Bitchat Android — Security Review

Commit: 92d07b22 (worktree: ~/.opencode/worktrees/bitchat-security-review) Scope: Exploitable bugs, privacy violations, tracking opportunities, DoS vectors. Method: Read-only static review of crypto/, noise/, identity/, mesh/, protocol/, net/, service/, nostr/, geohash/, ui/, features/, manifest & build config. All findings verified against source.


CRITICAL

C1. Noise session key material (DH shared secrets, chaining keys) written to logcat — ships in release

noise/southernstorm/protocol/SymmetricState.java:135-159 (also 98-104, 168-182) mixKey() logs the raw X25519 shared secret ("Input data"), current and new chaining keys in hex. proguard-rules.pro has no assumenosideeffects for android.util.Log, so this reaches the release APK. Anyone with the logcat transcript of a handshake can recompute split() outputs and fully decrypt the transport session. Release-blocking.

C2. Remote decompression bomb — pre-auth memory exhaustion via ~30-byte BLE write

protocol/BinaryProtocol.kt:429-454, protocol/CompressionUtil.kt:75-118 For compressed v2 packets the decoder reads a 4-byte attacker-controlled originalSize and immediately does ByteArray(originalSize) — up to 2 GiB per packet. The ratio guard is skipped when compressedSize == 0 and is 50,000:1 anyway (real deflate max ≈ 1032:1). Runs inline on the GATT callback path; any nearby unauthenticated BLE peer can OOM/crash the foreground service with tiny writes.


HIGH

H1. Replay-window bit shift direction is wrong — Noise transport replay protection broken

noise/NoiseSession.kt:69-106 Bit indexing is LSB-first, but the window-shift code right-shifts the byte string. A replayed captured frame (nonce 0) tests a zero bit and is accepted; only the single most-recent nonce is reliably blocked. A passive BLE observer can replay DMs, ACKs, and receipts.

H2. Announcement signing key not bound to Noise key — "first announce wins" peer-ID impersonation

mesh/AnnouncementIdentityValidator.kt:13-36, mesh/SecurityManager.kt:282-313, mesh/MessageHandler.kt:265-299 An attacker can replay a victim's Noise public key (→ victim's peerID) inside an announcement signed with the attacker's Ed25519 key. On fresh installs or for never-authenticated peers the forged binding wins, and the genuine victim's later announce is rejected as "key replacement".

H3. Decrypted private message content written to logcat

mesh/MessageHandler.kt:88 (also 125; nostr/NostrProtocol.kt:277) Log.d logs the first 30 chars of every decrypted E2E private message plus sender peerID. Not stripped in release. Defeats E2E guarantees for any logcat reader. Related: ui/DataManager.kt:161-196 dumps the full favorite/fingerprint social graph at startup (Low).

H4. E2E-encrypted DM content exposed in system notifications without lock-screen redaction

ui/NotificationManager.kt:224-246 Decrypted DM bodies (mesh + Nostr) are posted with full text in BigTextStyle/InboxStyle; no VISIBILITY_SECRET / lockscreen visibility set. Content renders on lock screen and to any notification-listener app; no opt-out.

H5. No signature verification on incoming public Nostr events — relay can forge any user

nostr/GeohashMessageHandler.kt:44-105, nostr/LocationNotesManager.kt:358-414, nostr/NostrRelayManager.kt:663-706 NostrEvent.isValidSignature() (exists, NostrEvent.kt:171) is never called for kind 20000/20001/1. A malicious/compromised relay (auto-selected from a daily third-party list, see M9) can inject events with arbitrary pubkeys — impersonating any user in any geohash channel, spoofing participants, caching attacker-chosen nicknames, registering DM aliases.

H6. Automatic delivery/read receipts over Nostr = online-presence & location oracle

nostr/NostrDirectMessageHandler.kt:134-138,165-175, nostr/NostrTransport.kt:316-384 Any decryptable gift-wrapped DM triggers an automatic signed DELIVERED ack. For geohash DMs the ack uses the public deterministic per-geohash identity, so anyone can probe a target's known geohash pubkey and confirm the device is online now, and chart activity patterns. No setting gates this.

H7. Building-precision geohash published publicly and permanently in location notes

geohash/LocationChannel.kt:8 (precision 8 ≈ 19×38 m), nostr/LocationNotesManager.kt:119-165 Kind-1 (relay-archived, persistent) notes carry the 8-char geohash, exact created_at, and an optional plaintext nickname, signed by a stable pubkey — a permanent public record "pubkey X was within ~30 m of this spot at time T". REQ #g filters also disclose precise location to relays.

H8. Deterministic per-geohash identities are stable forever — long-term passive location tracking

nostr/NostrIdentity.kt:137-177, ui/GeohashViewModel.kt:184-193 Geohash identity = HMAC(deviceSeed, geohash), never rotated. A passive observer of a channel can recognize returning users months later and assemble their full history in that channel; the plaintext ["n", nickname] tag (nostr/NostrProtocol.kt:167-169) links the same person across channels.

H9. Unbounded per-peer actor map and unlimited packet queues — memory/coroutine exhaustion

mesh/PacketProcessor.kt:43-95, mesh/BluetoothPacketBroadcaster.kt:122-124 A new coroutine actor with Channel.UNLIMITED is created per attacker-chosen senderID before any security validation, with no eviction. Spraying packets with random sender IDs grows live coroutines + unbounded channels forever.

H10. Unauthenticated packet types relayed mesh-wide with attacker-controlled TTL — flood/amplification

mesh/SecurityManager.kt:268-280, mesh/PacketRelayManager.kt:59-107,134-163 Signature verification enforced only for ANNOUNCE/MESSAGE/FILE_TRANSFER/LEAVE. FRAGMENT, REQUEST_SYNC, NOISE_* verify unconditionally and are relayed (unconditionally at TTL ≥ 4) with no rate limit. One BLE radio can make the entire mesh re-broadcast forged traffic, draining bandwidth and battery.

H11. Unsigned REQUEST_SYNC forces bulk re-broadcast — amplification

mesh/BluetoothMeshService.kt:671-676, sync/GossipSyncManager.kt:168-199 A spoofed REQUEST_SYNC with an empty (forgeable) GCS filter makes a victim dump its entire sync cache onto the radio; repeating keeps neighbors transmitting continuously. No rate limit or response budget.

H12. ANNOUNCE replay with TTL=7 forces Noise session teardown + fake "direct neighbor"

mesh/SecurityManager.kt:78-90, mesh/BluetoothMeshService.kt:586-621 Duplicate ANNOUNCEs are re-accepted at TTL ≥ 7; direct-link is inferred from TTL alone (excluded from the signature, attacker-settable). Replaying a victim's recent ANNOUNCE at TTL=7 repeatedly tears down the victim's Noise sessions, breaking in-flight DMs.


MEDIUM

M1. No low-order point / all-zero DH rejection — Noise spec violation, key-compromise downgrade

noise/southernstorm/protocol/HandshakeState.java:750-762,1034-1040,1050-1068 Only the all-zero ephemeral key is rejected; other low-order Curve25519 points pass, DH outputs never checked, remote static key unvalidated. A malicious identity with low-order keys yields all-zero DH outputs → publicly derivable session keys. SecureIdentityStateManager.validatePublicKey() (identity/…:403-417) is debug-only and its blocklist misses real low-order points.

M2. Unbounded crypto work + session state per forged handshake — crypto/memory DoS

noise/NoiseSessionManager.kt:228-252,312-316 Any frame from any spoofed peerID creates a responder session (~3 X25519 scalar mults) inside a @Synchronized method, with no cap on half-open responder sessions (staleness check only on the initiate path).

M3. Stable 8-byte peerID in BLE scan response — passive long-term tracking

mesh/BluetoothGattServerManager.kt:400-412, mesh/BluetoothMeshService.kt:54 The scan response embeds the truncated fingerprint of the persistent static Noise identity, surviving restarts and MAC rotation (by design, for dedup). Any passive sniffer can track a user across time/place. Nickname is broadcast in every ANNOUNCE; ANNOUNCE gossip TLVs disclose the user's neighbor graph.

M4. Fragment reassembly poisoning — cross-sender fragment-ID collision

mesh/FragmentManager.kt:32,202-263 Reassembly keyed by 8-byte fragment ID only (not sender). Fragments are unsigned; an attacker injects one colliding fragment to destroy a victim's in-flight (up to 1 MB) transfer. Bounded by existing caps; DoS/corruption, not forgery.

M5. Legacy plaintext Ed25519 private key can persist after "migration"

crypto/EncryptionService.kt:505-524 Old plaintext key is deleted only if the encrypted store doesn't already have one; otherwise the plaintext private key remains on disk indefinitely.

M6. Attacker-controlled image decode bombs / main-thread decode

ui/media/ImageMessageItem.kt:74, ui/media/FullScreenImageViewer.kt:75, ui/MessageComponents.kt:294-304 Received images decoded with BitmapFactory.decodeFile — no bounds check, no inSampleSize, on the main thread during composition. A small PNG with huge dimensions → instant OOM when the chat renders. readBytes() re-reads whole files on every recomposition.

M7. Received files auto-downloaded unencrypted; size limit only on send path

features/file/FileUtils.kt:194-263, nostr/NostrDirectMessageHandler.kt:191-194 Nostr DM path accepts up to 10 MB per message (AppConstants.kt:71); a malicious contact can fill cacheDir indefinitely. Filenames are sanitized (path traversal verified not exploitable).

M8. Exported MainActivity acts on attacker-supplied intent extras

AndroidManifest.xml:98-115, MainActivity.kt:828-894 Any app can fire intents with EXTRA_OPEN_PRIVATE_CHAT/EXTRA_PEER_ID to open arbitrary chat sheets and silently clear the victim's pending notifications, or trigger the verification sheet UI. QR payload itself is cryptographically validated — no verification forgery.

M9. Relay directory auto-updates daily from third-party GitHub CSV, unsigned/unpinned

nostr/RelayDirectory.kt:29,152-191 Compromise of permissionlesstech/georelays steers all users to attacker relays → precise #g filters + forged-event injection (H5).

M10. Geohash subscription filters & geo-nearest relay selection leak location to relays/observers

nostr/NostrRelayManager.kt:131-147, nostr/RelayDirectory.kt:88-106, nostr/NostrFilter.kt:37-71 Each relay learns subscribed cells; #p DM filters reveal owned pubkeys. Mitigated: Tor ON by default, fail-closed proxy config (net/ArtiTorManager.kt:151,224).

M11. Unbounded outbound Nostr message queue — never drained, re-sent on reconnect

nostr/NostrRelayManager.kt:107,286-288,862-871 messageQueue entries are never removed after send; every relay reconnect re-sends full history (duplicate gift wraps, memory growth, extra metadata).

M12. Unbounded identity-keyed caches — memory DoS by malicious relay (compounds H5)

nostr/GeohashRepository.kt:22-29,61,104-118 geohashParticipants, geoNicknames, etc. grow per unique pubkey with no eviction; forged events from unlimited fresh pubkeys exhaust memory. No WebSocket frame size cap (NostrRelayManager.kt:874-876).

M13. "NIP-44" DM encryption omits padding — exact plaintext length leakage; not real NIP-44

nostr/NostrCrypto.kt:211-260,267-293 Raw XChaCha20-Poly1305 over unpadded UTF-8; relays/observers see exact DM lengths (receipt vs message vs file) and it's incompatible with real NIP-44 clients. AEAD itself sound.

M14. Peer-table flooding with throwaway identities

mesh/PeerManager.kt:93,228-240,551-564 No cap on the peers map; cheap self-signed ANNOUNCEs create verified entries and can each trigger Noise handshakes. 3-minute sweep is the only bound.

M15. No rate limiting on GATT writes / inbound packet processing

mesh/BluetoothGattServerManager.kt:232-277, mesh/BluetoothGattClientManager.kt:644-656 Every write is fully decoded/verified with no per-connection or global rate limit; combined with H9 guarantees backlog growth.

M16. Two independent persistent Ed25519 signing identities per device

crypto/EncryptionService.kt:485-503 vs identity/SecureIdentityStateManager.kt:145-195 Different keys in different pref files; same device presents two signing identities; panic wipe doesn't rotate both. Also: EncryptionService.sign() returns an empty signature and verify() ignores its inputs entirely (EncryptionService.kt:226-242) — a latent trap.


LOW

  • L1. Metadata leakage: only NOISE_* frames padded; public MESSAGE/ANNOUNCE/FILE_TRANSFER leak exact sizes; every packet carries ms wall-clock timestamps (mesh/BLEPacketPaddingPolicy.kt:11-17, protocol/BinaryProtocol.kt:42-43,76).
  • L2. Signed MESSAGE/FILE_TRANSFER replayable after 5-min dedup expiry — no freshness check (mesh/SecurityManager.kt:55-73,101).
  • L3. Silent identity regeneration on key-store corruption — masks tampering, destroys identity on transient Keystore failure (crypto/EncryptionService.kt:466-483, noise/NoiseEncryptionService.kt:85-119).
  • L4. Channel KDF salt = channel name; PBKDF2 mislabeled as "Argon2id" in comments; passwords retained in memory; dead plaintext channel-key-sharing packet code (noise/NoiseChannelEncryption.kt:148-168,203-243).
  • L5. Reverse geocoding sends precise GPS to OSM Nominatim (or Google Fused) with app-identifying UA — rides Tor when on (geohash/OpenStreetMapGeocoderProvider.kt:21-29).
  • L6. Persistent account Nostr identity links all DM activity; #p filters announce pubkey ownership to relays — mitigated by randomized gift-wrap timestamps (nostr/NostrIdentity.kt:108-128).
  • L7. Predictable subscription IDs (sub-<millis>-<rand>) aid per-session correlation (nostr/NostrRelayManager.kt:809-811).
  • L8. NIP-17 validation gaps: rumor kind not checked to be 14; replay window uses attacker-controlled created_at. Sender spoofing inside gift wraps is correctly prevented (nostr/NostrProtocol.kt:77-96).
  • L9. No FLAG_SECURE anywhere; Recents preview leaks chats on API < 33 (MainActivity.kt:84-86).
  • L10. Clipboard copies not marked sensitive (ui/ChatUserSheet.kt:87, ui/SecurityVerificationSheet.kt:411).
  • L11. WebView with JS + file access + unescaped geohash interpolation into evaluateJavascript — activity not exported, limited impact (ui/GeohashPickerActivity.kt:104-148).
  • L12. packet.recipientID != SpecialRecipients.BROADCAST is ByteArray reference comparison — always true (mesh/BluetoothPacketBroadcaster.kt:340).
  • L13. GATT server ignores preparedWrite/offset; force-unwrap NPEs on hot paths degrade gracefully (mesh/BluetoothGattServerManager.kt:232-239, mesh/PacketProcessor.kt:127).

Verified positives

  • SecureRandom everywhere; no weak RNG.
  • Keys in EncryptedSharedPreferences (AES256-GCM Keystore master key); allowBackup="false" + backup rules.
  • No plaintext message DB; messages in-memory only; message IDs in encrypted prefs.
  • PeerID = SHA-256(static key)[:8] with session establishment refusing mismatches; session-generation binding defeats re-handshake downgrade.
  • AEAD verify-before-decrypt, constant-time tag check, nonce-wraparound enforcement, rekey limits, zeroization on destroy.
  • NIP-17 seal signature verified + seal.pubkey == rumor.pubkey enforced; Schnorr nonces use SecureRandom.
  • Tor fail-closed (proxy set before bootstrap; clients rebuilt on mode change); no telemetry/analytics SDKs; all traffic wss/https.
  • Filename sanitization blocks path traversal; FileProvider per-URI grants; no auto-opening URLs; links require explicit tap and are coerced to https.
  • Fragment caps (256/ID, 1 MB/set, 4 MB global, 30 s timeout); dedup caches bounded; connection limits + backoff; presence heartbeats only at city-level precision with jitter.

Top remediations (priority order)

  1. Strip all key/content logging (SymmetricState, MessageHandler.kt:88); add ProGuard assumenosideeffects for android.util.Log. (C1, H3)
  2. Hard-cap originalSize in CompressionUtil.decompress (~12 MB); tighten ratio to ≈1032:1. (C2)
  3. Fix replay-window shift direction + unit tests; reject low-order points / all-zero DH in mixDH. (H1, M1)
  4. Verify isValidSignature() on all incoming Nostr events; pin/sign the relay directory. (H5, M9)
  5. Cross-sign Ed key with Noise key (proof-of-possession) in announcements. (H2)
  6. Bound PacketProcessor actor map (validate before actor creation), bound queues, authenticate/rate-limit FRAGMENT & REQUEST_SYNC relay, budget sync responses. (H9H11)
  7. Key fragment reassembly by (senderID, fragmentID); rotate the advertised peerID. (M4, M3)
  8. Gate auto delivery/read receipts behind a privacy setting; warn about location-note precision/permanence. (H6, H7)
  9. Lock-screen-redact DM notifications; image decode bounds checks off main thread. (H4, M6)
  10. Bound Nostr messageQueue (drain after send) and GeohashRepository caches. (M11, M12)