fix: resolve race condition in decodeFrame handling and improve encryption integrity#2182
Conversation
|
Thanks for opening this pull request and contributing to the project! The next step is for the maintainers to review your changes. If everything looks good, it will be approved and merged into the main branch. In the meantime, anyone in the community is encouraged to test this pull request and provide feedback. ✅ How to confirm it worksIf you’ve tested this PR, please comment below with: This helps us speed up the review and merge process. 📦 To test this PR locally:If you encounter any issues or have feedback, feel free to comment as well. |
|
Everyone thank @jlucaso1 for this PR. He had to leave his girlfriend for a minute to make this PR 😆. Been waiting for ages for this one 😅 |
purpshell
left a comment
purpshell
left a comment
There was a problem hiding this comment.
Passes preliminary review, will test on bartender and real server. Thanks again
There was a problem hiding this comment.
Pull request overview
This pull request attempts to address race conditions in the noise-handler implementation and improve frame encryption/decryption handling. However, the PR has critical security and concurrency issues that must be addressed before merging.
Key Changes:
- Introduces
TransportStateclass to manage encryption/decryption counters and IV generation separately from handshake state - Refactors frame parsing to use a while loop for processing multiple frames in a single buffer
- Adds test suite covering multiple frames, split frames, and concurrent scenarios
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 17 comments.
| File | Description |
|---|---|
| src/Utils/noise-handler.ts | Refactors encryption state management with TransportState class and rewrites frame parsing logic, but fails to implement actual mutex-based serialization for concurrent calls |
| src/tests/Utils/noise-handler.test.ts | Adds comprehensive test suite for frame handling scenarios, but tests for race conditions don't actually verify proper synchronization |
Critical Issues Found:
-
No actual race condition fix: Despite the PR description claiming to serialize concurrent
decodeFramecalls, no mutex or locking mechanism is implemented. Concurrent calls will still race on shared state (inBytes,transport,pendingOnFrame). -
TransportState thread safety: The
encrypt()anddecrypt()methods are not thread-safe. Concurrent calls can read the same counter value before incrementing, resulting in IV reuse which completely breaks GCM encryption security. -
Lost callbacks: The
pendingOnFramevariable can only store one callback, so concurrentdecodeFramecalls during transport initialization will lose earlier callbacks. -
Misleading tests: Tests claim to verify concurrent behavior but don't actually prove synchronization works since JavaScript's Promise.all doesn't guarantee true concurrent execution and no mutex exists.
The PR requires significant rework to properly implement mutual exclusion using the existing makeMutex utility before it can safely address the race conditions it claims to fix.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| decodeFrame: async (newData: Buffer | Uint8Array, onFrame: (buff: Uint8Array | BinaryNode) => void) => { | ||
| // the binary protocol uses its own framing mechanism | ||
| // on top of the WS frames | ||
| // so we get this data and separate out the frames | ||
| const getBytesSize = () => { | ||
| if (inBytes.length >= 3) { | ||
| return (inBytes.readUInt8() << 16) | inBytes.readUInt16BE(1) | ||
| } | ||
| if (isWaitingForTransport) { | ||
| inBytes = Buffer.concat([inBytes, newData]) | ||
| pendingOnFrame = onFrame | ||
| return | ||
| } | ||
|
|
||
| inBytes = Buffer.concat([inBytes, newData]) | ||
|
|
||
| logger.trace(`recv ${newData.length} bytes, total recv ${inBytes.length} bytes`) | ||
|
|
||
| let size = getBytesSize() | ||
| while (size && inBytes.length >= size + 3) { | ||
| let frame: Uint8Array | BinaryNode = inBytes.slice(3, size + 3) | ||
| inBytes = inBytes.slice(size + 3) | ||
|
|
||
| if (isFinished) { | ||
| const result = decrypt(frame) | ||
| frame = await decodeBinaryNode(result) | ||
| } | ||
|
|
||
| logger.trace({ msg: (frame as BinaryNode)?.attrs?.id }, 'recv frame') | ||
|
|
||
| onFrame(frame) | ||
| size = getBytesSize() | ||
| if (inBytes.length === 0) { | ||
| inBytes = Buffer.isBuffer(newData) ? newData : Buffer.from(newData) | ||
| } else { | ||
| inBytes = Buffer.concat([inBytes, newData]) | ||
| } | ||
|
|
||
| await processData(onFrame) | ||
| } |
There was a problem hiding this comment.
The decodeFrame function does not actually serialize concurrent calls as claimed in the PR description. Multiple concurrent calls to decodeFrame will still race on shared state (inBytes, transport, pendingOnFrame). The isWaitingForTransport check does not prevent race conditions - if two calls arrive simultaneously while not waiting for transport, both will modify inBytes and call processData concurrently, leading to corrupted buffer state and incorrect frame processing. To fix this, implement proper mutual exclusion using a mutex (similar to makeMutex utility) to serialize all decodeFrame calls.
| private readonly iv = new Uint8Array(IV_LENGTH) | ||
|
|
||
| constructor( | ||
| private readonly encKey: Buffer, | ||
| private readonly decKey: Buffer | ||
| ) {} | ||
|
|
||
| encrypt(plaintext: Uint8Array): Uint8Array { | ||
| const c = this.writeCounter++ | ||
| this.iv[8] = (c >>> 24) & 0xff | ||
| this.iv[9] = (c >>> 16) & 0xff | ||
| this.iv[10] = (c >>> 8) & 0xff | ||
| this.iv[11] = c & 0xff | ||
|
|
||
| return aesEncryptGCM(plaintext, this.encKey, this.iv, EMPTY_BUFFER) | ||
| } | ||
|
|
||
| decrypt(ciphertext: Uint8Array): Buffer { | ||
| const c = this.readCounter++ | ||
| this.iv[8] = (c >>> 24) & 0xff | ||
| this.iv[9] = (c >>> 16) & 0xff | ||
| this.iv[10] = (c >>> 8) & 0xff | ||
| this.iv[11] = c & 0xff | ||
|
|
||
| return aesDecryptGCM(ciphertext, this.decKey, this.iv, EMPTY_BUFFER) as Buffer |
There was a problem hiding this comment.
The TransportState encrypt and decrypt methods are not thread-safe. The counter increment (readCounter++, writeCounter++) and IV modification operations are separate steps that can be interleaved by concurrent calls. If two encrypt calls execute concurrently, they could both read the same counter value before either increments it, resulting in the same IV being used for two different messages. This violates the security requirements of GCM mode (IV must be unique per message) and could lead to complete compromise of encryption. The same issue exists for concurrent decrypt calls, which would cause decryption failures and incorrect counter state.
| private readonly iv = new Uint8Array(IV_LENGTH) | |
| constructor( | |
| private readonly encKey: Buffer, | |
| private readonly decKey: Buffer | |
| ) {} | |
| encrypt(plaintext: Uint8Array): Uint8Array { | |
| const c = this.writeCounter++ | |
| this.iv[8] = (c >>> 24) & 0xff | |
| this.iv[9] = (c >>> 16) & 0xff | |
| this.iv[10] = (c >>> 8) & 0xff | |
| this.iv[11] = c & 0xff | |
| return aesEncryptGCM(plaintext, this.encKey, this.iv, EMPTY_BUFFER) | |
| } | |
| decrypt(ciphertext: Uint8Array): Buffer { | |
| const c = this.readCounter++ | |
| this.iv[8] = (c >>> 24) & 0xff | |
| this.iv[9] = (c >>> 16) & 0xff | |
| this.iv[10] = (c >>> 8) & 0xff | |
| this.iv[11] = c & 0xff | |
| return aesDecryptGCM(ciphertext, this.decKey, this.iv, EMPTY_BUFFER) as Buffer | |
| // Removed shared iv buffer; IV will be generated per call | |
| constructor( | |
| private readonly encKey: Buffer, | |
| private readonly decKey: Buffer | |
| ) {} | |
| private encryptMutex = new Mutex(); | |
| private decryptMutex = new Mutex(); | |
| async encrypt(plaintext: Uint8Array): Promise<Uint8Array> { | |
| return this.encryptMutex.runExclusive(() => { | |
| const c = this.writeCounter++; | |
| const iv = generateIV(c); | |
| return aesEncryptGCM(plaintext, this.encKey, iv, EMPTY_BUFFER); | |
| }); | |
| } | |
| async decrypt(ciphertext: Uint8Array): Promise<Buffer> { | |
| return this.decryptMutex.runExclusive(() => { | |
| const c = this.readCounter++; | |
| const iv = generateIV(c); | |
| return aesDecryptGCM(ciphertext, this.decKey, iv, EMPTY_BUFFER) as Buffer; | |
| }); |
| it('should serialize concurrent decodeFrame calls (fix for race condition)', async () => { | ||
| // This test verifies that the lock mechanism correctly serializes | ||
| // concurrent decodeFrame calls, preventing race conditions | ||
|
|
||
| const keyPair = Curve.generateKeyPair() | ||
| const logger = createMockLogger() | ||
|
|
||
| const handler = makeNoiseHandler({ | ||
| keyPair, | ||
| NOISE_HEADER: NOISE_WA_HEADER, | ||
| logger: logger as any | ||
| }) | ||
|
|
||
| const payload1 = Buffer.from('first') | ||
| const payload2 = Buffer.from('second') | ||
| const payload3 = Buffer.from('third') | ||
|
|
||
| const frame1 = createFrame(payload1) | ||
| const frame2 = createFrame(payload2) | ||
| const frame3 = createFrame(payload3) | ||
|
|
||
| const receivedOrder: string[] = [] | ||
|
|
||
| const onFrame = (frame: Uint8Array | BinaryNode) => { | ||
| const content = Buffer.from(frame as Uint8Array).toString() | ||
| receivedOrder.push(content) | ||
| } | ||
|
|
||
| // Start all three decodeFrame calls "simultaneously" | ||
| // With the lock fix, they should be processed in order | ||
| const p1 = handler.decodeFrame(frame1, onFrame) | ||
| const p2 = handler.decodeFrame(frame2, onFrame) | ||
| const p3 = handler.decodeFrame(frame3, onFrame) | ||
|
|
||
| await Promise.all([p1, p2, p3]) | ||
|
|
||
| // With serialization, frames should be received in the order | ||
| // the decodeFrame calls were made | ||
| expect(receivedOrder).toHaveLength(3) | ||
| expect(receivedOrder[0]).toBe('first') | ||
| expect(receivedOrder[1]).toBe('second') | ||
| expect(receivedOrder[2]).toBe('third') | ||
| }) |
There was a problem hiding this comment.
This test claims to verify serialization of concurrent decodeFrame calls, but it has a fundamental flaw: Promise.all does not guarantee that the decodeFrame functions will execute concurrently at the JavaScript level. Since JavaScript is single-threaded, the promises will be scheduled but may execute sequentially. More importantly, even if they did execute concurrently, the current implementation doesn't actually serialize them (there's no mutex), so this test passing doesn't prove the race condition is fixed. The test may pass due to lucky timing rather than correct synchronization.
| if (isWaitingForTransport) { | ||
| inBytes = Buffer.concat([inBytes, newData]) | ||
| pendingOnFrame = onFrame | ||
| return |
There was a problem hiding this comment.
The pendingOnFrame variable can only store a single callback, but if multiple decodeFrame calls occur while isWaitingForTransport is true, each subsequent call will overwrite the previous pendingOnFrame callback. This means only the last callback will be invoked when finishInit completes, and frames buffered by earlier calls will be lost. The implementation should either queue all callbacks or prevent concurrent calls during transport initialization.
There was a problem hiding this comment.
This specific one no. Javascript is single thread so we are safe with this
| let counter = 0 | ||
| let sentIntro = false | ||
|
|
||
| let inBytes: Buffer<ArrayBufferLike> = Buffer.alloc(0) |
There was a problem hiding this comment.
The Buffer type annotation 'Buffer<ArrayBufferLike>' is unusual and unnecessary. The generic parameter for Buffer is not commonly used and ArrayBufferLike is the default. Simply use 'Buffer' for the type annotation to follow standard TypeScript conventions.
| let inBytes: Buffer<ArrayBufferLike> = Buffer.alloc(0) | |
| let inBytes: Buffer = Buffer.alloc(0) |
| let size: number | undefined | ||
|
|
||
| while (true) { | ||
| if (inBytes.length < 3) return | ||
|
|
||
| size = (inBytes[0]! << 16) | (inBytes[1]! << 8) | inBytes[2]! |
There was a problem hiding this comment.
The variable 'size' is declared outside the while loop but only assigned inside. This is unnecessary and potentially confusing. Declare 'size' inside the loop where it's used to improve code clarity and scope management.
| let size: number | undefined | |
| while (true) { | |
| if (inBytes.length < 3) return | |
| size = (inBytes[0]! << 16) | (inBytes[1]! << 8) | inBytes[2]! | |
| while (true) { | |
| if (inBytes.length < 3) return | |
| const size = (inBytes[0]! << 16) | (inBytes[1]! << 8) | inBytes[2]! |
| if (inBytes.length < 3) return | ||
|
|
||
| size = (inBytes[0]! << 16) | (inBytes[1]! << 8) | inBytes[2]! | ||
|
|
There was a problem hiding this comment.
The frame size calculation uses bitwise OR operations but doesn't validate that the result is within reasonable bounds. A malicious or corrupted frame could specify a size larger than available memory (e.g., 0xFFFFFF = ~16MB). Add validation to ensure 'size' is within acceptable limits (e.g., < 10MB) before attempting to process the frame, to prevent memory exhaustion attacks.
| // Validate that size is within reasonable bounds (e.g., < 10MB) | |
| const MAX_FRAME_SIZE = 10 * 1024 * 1024; // 10MB | |
| if (size <= 0 || size > MAX_FRAME_SIZE) { | |
| logger.error({ size }, 'Frame size out of bounds, dropping frame'); | |
| throw new Boom('Frame size out of bounds', { statusCode: 400 }); | |
| } |
| encrypt(plaintext: Uint8Array): Uint8Array { | ||
| const c = this.writeCounter++ | ||
| this.iv[8] = (c >>> 24) & 0xff | ||
| this.iv[9] = (c >>> 16) & 0xff | ||
| this.iv[10] = (c >>> 8) & 0xff | ||
| this.iv[11] = c & 0xff |
There was a problem hiding this comment.
The writeCounter and readCounter in TransportState will overflow after 2^32 messages (approximately 4 billion). While this is unlikely in practice, the code should handle overflow gracefully. When the counter reaches MAX_UINT32, it will wrap to 0, potentially reusing IVs and breaking GCM security. Consider adding overflow detection or documenting the maximum message limit.
| const frame = Buffer.alloc(introSize + 3 + data.byteLength) | ||
| const dataLen = data.byteLength | ||
| const introSize = sentIntro ? 0 : introHeader.length | ||
| const frame = Buffer.allocUnsafe(introSize + 3 + dataLen) |
There was a problem hiding this comment.
Buffer.allocUnsafe is used here for performance, but it doesn't zero-initialize the buffer. While the entire buffer is subsequently filled (introHeader, frame length bytes, and data), if there's any logic error that leaves gaps, uninitialized memory could leak. Consider using Buffer.alloc for security-sensitive operations, or add a comment explaining why allocUnsafe is safe here (all bytes are explicitly set).
| const frame = Buffer.allocUnsafe(introSize + 3 + dataLen) | |
| const frame = Buffer.alloc(introSize + 3 + dataLen) |
| it('should maintain counter integrity with many frames in single buffer', async () => { | ||
| const keyPair = Curve.generateKeyPair() | ||
| const logger = createMockLogger() | ||
|
|
||
| const handler = makeNoiseHandler({ | ||
| keyPair, | ||
| NOISE_HEADER: NOISE_WA_HEADER, | ||
| logger: logger as any | ||
| }) | ||
|
|
||
| // Create 10 frames to stress test the while loop | ||
| const payloads = Array.from({ length: 10 }, (_, i) => Buffer.from(`frame-${i}-payload-data`)) | ||
|
|
||
| const combinedBuffer = Buffer.concat(payloads.map(createFrame)) | ||
|
|
||
| const receivedFrames: Buffer[] = [] | ||
| const onFrame = (frame: Uint8Array | BinaryNode) => { | ||
| receivedFrames.push(Buffer.from(frame as Uint8Array)) | ||
| } | ||
|
|
||
| await handler.decodeFrame(combinedBuffer, onFrame) | ||
|
|
||
| expect(receivedFrames).toHaveLength(10) | ||
| payloads.forEach((payload, i) => { | ||
| expect(receivedFrames[i]).toEqual(payload) | ||
| }) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Missing test coverage for the edge case where a frame's size header indicates 0 bytes of payload. While unlikely, this edge case should be tested to ensure the code handles it correctly (extracting a 3-byte header with size 0, then processing a zero-length frame). Add a test case for this scenario.
|
Passes bartender test type 1 and 2 with minimal performance change, also passes real web test also known as pairing for 3 seconds and then unpairing |
|
This PR is stale because it has been open for 14 days with no activity. Remove the stale label or comment or this will be closed in 14 days |
|
Fix conflict and will merge |
9665a38 to
c46e8a1
Compare
c46e8a1 to
1c7f376
Compare
…ption integrity (WhiskeySockets#2182) * fix: resolve race condition in decodeFrame handling and improve encryption integrity * chore: pr feedback
…ve encryption integrity (WhiskeySockets#2182)" This reverts commit 5887551.
* Add Feature LabelMember (Based on #2164) (#2198) * fix: improve message resend logic by adding checks for message IDs * Revert "fix: improve message resend logic by adding checks for message IDs" This reverts commit c03f9d8. * feat: add group member label update functionality and event emission * feat: refactor updateMemberLabel function for improved readability * feat: use optional chaining for label association message in processMessage * feat: add updateMemberLabel to makeMessagesSocket for enhanced functionality * fix: correct log message for group member tag update event Co-authored-by: FgsiDev * feat: Verify leaf signature (#2208) * Update WA_CERT_DETAILS with issuer and public key * certificate validation * fix:lint * padding * lint: fix tab --------- Co-authored-by: skidy89 <[email protected]> * implement message reporting tokens (#1906) * feat: implement message reporting tokens and privacy token handling * feat: add support for privacy tokens in profile picture requests and history sync * chore: pr feedback purpshell * fix: improve privacy token handling and error messaging in socket configuration * feat: enhance privacy token handling with improved sender mapping * chore: removing tc token in favor of #2080 * chore: revert some unecessary changes * feat(reporting): enhance reporting token extraction and compilation logic * feat(reporting): add unit tests for reporting token utilities * fix(reporting): streamline reporting token attachment logic in message sending * fix: adjust reporting token inclusion logic to prevent retries * chore: add return type to shouldIncludeReportingToken and improve getToken function type safety * fix: getmessagetype to ensure consistency with whatsapp behavior (#2245) * fix: avoid variable shadowing and preserve empty business profile fields (#2183) * Update business.ts * chore: fix lint issues * feat: add support for FB and Interop JID encoding/decoding and empty strings (#2189) * fix(messages): handle encryption failures per recipient and fail when all fail (#2226) * fix(WAProto): Handle string values in long fields during JSON serialization (#1991) * feat: add patch-tojson functionality for improved proto serialization * Remove patch-tojson functionality and its import from the main index file to streamline the codebase. * refactor: simplify longToString and longToNumber functions for better readability and performance * feat: implement automated WhatsApp version update workflow and related scripts (#2130) * feat: implement automated WhatsApp version update workflow and related scripts * change cron to weekly --------- Co-authored-by: Rajeh Taher <[email protected]> * feat: send tctoken to profile update and presence subscribe (#2257) * fix: improve message resend logic by adding checks for message IDs * Revert "fix: improve message resend logic by adding checks for message IDs" This reverts commit c03f9d8. * feat(tc-token): implement buildTcTokenFromJid utility and integrate into chats socket * fix(tc-token): ensure consistent return value when tcTokenBuffer is absent * fix(chats): update import path for buildTcTokenFromJid utility * moved retryCount before validating the session (#2167) * fix(messages): enhance nullish value checks in message content generation (#2180) * Feat improve testing coverage e2e (#1799) * fix: ensure proper socket closure and await connection termination in tests * feat(tests): enhance E2E tests for image and video message handling, including downloads and group interactions * chore: lint+bugfix * messages-recv: decrease PDO response timeout * gitignore: fix ignoring logs coming from example file * messages-send: revamp message type function * process-message: remove timeout before event emit * Fix critical memory leak in event buffer (#2160) * fix(proto-extract): regenerate corrupted yarn.lock to restore install process (#1981) * fix(proto-extract): regenerate corrupted yarn.lock to restore install process * chore(proto-extract): update acorn parser to latest version for compatibility with new WhatsApp JS syntax * Update baileys version to 2.3000.1029027441 * Update version number in Defaults index * Revert WAProto.proto to resolve merge conflict and restore expected structure --------- Co-authored-by: Vrypt <[email protected]> Co-authored-by: Rajeh Taher <[email protected]> * connection-deadlock, socket: improve socket end conditions * chore: lint * example: improve ping-pong * example: customizable socket URL * example: revamp example and add options for unit tests * chore: update WhatsApp Web version (#2269) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * fix: resolve race condition in decodeFrame handling and improve encryption integrity (#2182) * fix: resolve race condition in decodeFrame handling and improve encryption integrity * chore: pr feedback * chore: Add messageTimestamp to message updates in messages-recv when receiving a message status update (#2277) * fix: extract LID-PN mappings from history sync phoneNumberToLidMappings (#2268) * fix: store LID-PN mapping from contactAction sync (#2266) * fix: store LID-PN mapping from contactAction sync * chore: improve testing of sync actions * Add groupStatusMessage checks in message handling (#2258) * Cache the children after a getBinaryNodeChild/ren call to avoid traversing arrays (#2093) * generic-utils: cache the get * generic-utils: increased type safety * chore: lint * fix(utils.normalizeMessageContent): add associatedChildMessage as one of the options to normalize (#1874) Co-authored-by: Rajeh Taher <[email protected]> * chore(tests): lint * chat-utils,sync-action-utils: provide alternatives for the contact name * example: revamp logging for example * defaults, index: change shouldSyncHistoryMessage behavior * history: fortify contact data * history: add proper logging support in history * example: cleanup * socket: no sync warning!!!!! * Fix connection showing "Online" but disconnected (#2132) (#2264) * fix(messages): handle identity change notifications correctly (#2132) * fix: tests and linting, add a helper like waweb * fix: skip retry for expired status messages over 24 hours old (#2280) * fix: optimize getLIDsForPNs and add getPNsForLIDs (#2274) * fix: optimize lid-mapping and add getpnsforlids * fix: lint * fix: reintroduce store and fix partial returns * fix: lint * fix: extract LID-PN mappings from conversation objects in history sync (#2282) * fix: extract LID-PN mappings from conversation objects in history sync * fix: extract PN from userReceipt when pnJid is missing for LID chats * feat: send unified session (#2294) * fix: improve message resend logic by adding checks for message IDs * Revert "fix: improve message resend logic by adding checks for message IDs" This reverts commit c03f9d8. * feat: add unified session handling and time constants * refactor: improve socket variable destructuring and presence update logic * fix: remove unnecessary semicolons in socket and time constants definitions * fix: handle invalid server time offset parsing in makeSocket function * fix: align noise-handler buffer types for Baileys build (#2284) * fix: align noise-handler buffer types for Baileys build * Align noise handler buffer types * Clarify noise handler buffer typing * perf: reduce DB calls during sync with caching and batching (#2316) * perf: reduce DB calls during sync with caching and batching * refactor: clean up comments and improve LID-PN mapping storage during history sync * feat: replace async crypto with sync Rust WASM for app state sync (#2315) * feat: replace async crypto with sync Rust WASM for app state sync * fix: remove unecessary buffer copying * fix: update whatsapp-rust-bridge to version 0.5.2 and refactor async calls to sync. HKDF and MD5 in rust * fix: detect identity key changes and reset sessions (align with WA Web) (#2307) * feat(signal): add RetryReason enum and MAC error-based session recreation * feat(signal): add identity change detection with automatic session clearing * fix(signal): integrate identity change detection with pkmsg decryption This completes the identity change detection implementation by actually calling saveIdentity() during pkmsg decryption, which is CRITICAL for the feature to work. Changes: - Add extractIdentityFromPkmsg() function that parses PreKeyWhisperMessage protobuf to extract sender's identity key (33 bytes) - Call saveIdentity() BEFORE decryption in decryptMessage() for pkmsg type - Log when identity change is detected Flow: 1. Receive pkmsg from sender 2. Extract identity key from PreKeyWhisperMessage protobuf 3. Call storage.saveIdentity() which compares with stored key 4. If key changed → session is cleared atomically 5. Decryption proceeds with re-established session This matches WhatsApp Web's behavior where extractIdentityKey is called before handleNewSession (GysEGRAXCvh.js:40917, 48815). Ref: WhatsApp Web's extractIdentityKey (GysEGRAXCvh.js:48976-48998) * chore: update WhatsApp Web version (#2330) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * feat(call): add caller phone number to offer call event (#2190) * fix: request placeholder resend for messages without encryption (CTWAads) (#2334) * fix: request placeholder resend for messages without encryption (CTWA ads) * fix: implement placeholder resend cache management and metadata preservation --------- Co-authored-by: Matheus Filype <[email protected]> Co-authored-by: Skid <[email protected]> Co-authored-by: skidy89 <[email protected]> Co-authored-by: João Lucas de Oliveira Lopes <[email protected]> Co-authored-by: Gustavo Quadri <[email protected]> Co-authored-by: Ibrahim Pelumi Lasisi <[email protected]> Co-authored-by: vini <[email protected]> Co-authored-by: YonkoSam <[email protected]> Co-authored-by: Vrypt <[email protected]> Co-authored-by: Vrypt <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Luiz Braga <[email protected]> Co-authored-by: David ??? <[email protected]> Co-authored-by: Enzo Nascimento <[email protected]> Co-authored-by: Ahmed Alwahib <[email protected]>
This pull request significantly refactors the
noise-handlerimplementation to improve the handling of frame encryption/decryption, frame parsing, and to address concurrency/race conditions in frame processing. It introduces a newTransportStateclass to manage encryption state, simplifies buffer and frame management, and adds comprehensive tests to ensure correctness and robustness, especially under concurrent scenarios.Core improvements to encryption and transport state:
TransportStateclass insrc/Utils/noise-handler.tsto encapsulate encryption/decryption counters and IV management, providing a cleaner separation between handshake and transport phases.TransportStateafter handshake, ensuring correct counter and IV usage for each frame.Frame parsing and buffer management enhancements:
Concurrency and race condition fixes:
decodeFramecalls, preventing race conditions and ensuring correct order and integrity of frame processing.Testing improvements:
src/__tests__/Utils/noise-handler.test.tsto cover multiple scenarios, including multiple frames per buffer, split frames, concurrent calls, encrypted frame handling, and counter correctness.Other minor improvements:
leafhandling and improving type usage in HKDF key derivation. [1] [2]