fix: replace ephemeral sodium_malloc with Buffer.alloc in encrypt/decrypt hot path#258
Conversation
Replace uses of sodium.sodium_malloc with Node Buffer.alloc across EncryptemClient.ts, EncryptemServer.ts, and KeyPairGenerator.ts. Allocations for auth keys, nonces, cipher/decrypted buffers, signatures, and seeds now use Buffer.alloc while preserving existing sodium.randombytes_* and crypto_* calls. Also fix an incorrect reference in EncryptemServer.generateNonce to use this.sodium.crypto_box_NONCEBYTES. No functional changes to cryptographic logic; this simplifies memory handling by using Node Buffers instead of libsodium's sodium_malloc API.
There was a problem hiding this comment.
Pull request overview
This PR addresses Node.js crashes under sustained P2P traffic by replacing frequent sodium_malloc allocations (which use mmap/guard pages and can exhaust vm.max_map_count) with regular Buffer.alloc in the encrypt/decrypt/authenticate/verify hot paths, keeping sodium_malloc for long-lived key material.
Changes:
- Replace per-message
sodium_mallocallocations withBuffer.allocin Encryptem client/server encryption, decryption, auth, and signing paths. - Replace ephemeral signing/seed buffers in
KeyPairGeneratorwithBuffer.allocwhile leaving long-lived keypair allocations onsodium_malloc.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/src/poc/networking/encryptem/KeyPairGenerator.ts |
Switch ephemeral signature/seed allocations to Buffer.alloc. |
src/src/poc/networking/encryptem/EncryptemServer.ts |
Remove per-message sodium_malloc allocations in server encrypt/decrypt/auth/sign flows. |
src/src/poc/networking/encryptem/EncryptemClient.ts |
Remove per-message sodium_malloc allocations in client encrypt/decrypt/auth/sign flows. |
Comments suppressed due to low confidence (2)
src/src/poc/networking/encryptem/KeyPairGenerator.ts:101
signatureis allocated with a fixed size (Buffer.alloc(sodium.crypto_sign_BYTES)), so the laterif (!signature.byteLength)check can never trigger and doesn’t validate signing success. Consider removing that check or replacing it with a real success/failure signal (e.g., relying on sodium-native throwing, or checking a boolean return if the binding provides one).
const signature = Buffer.alloc(sodium.crypto_sign_BYTES);
sodium.crypto_sign_detached(
signature,
Buffer.from(data.buffer, data.byteOffset, data.byteLength),
Buffer.from(privateKey.buffer, privateKey.byteOffset, privateKey.byteLength),
src/src/poc/networking/encryptem/EncryptemServer.ts:206
crypto_box_open_easyreturns a success boolean (MAC check). The return value is currently ignored, so if decryption fails (e.g., nonce tampering), the function may still proceed and return the (zero-filled)decryptedMessageas long as the signature verifies. Capture and check the return value and fail/return null when it’s false (before using/returning the plaintext).
this.sodium.crypto_box_open_easy(
decryptedMessage,
cipher,
nonce,
senderPublicKey,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const decryptedMessage = Buffer.alloc(cipher.length - this.sodium.crypto_box_MACBYTES); | ||
|
|
||
| if (!this.verifyAuth(auth, signature)) { | ||
| throw new Error('[Client] Bad AHEAD authentication.'); |
There was a problem hiding this comment.
The subsequent crypto_box_open_easy(...) call’s return value isn’t checked. If decryption fails (e.g., altered nonce/MAC), the code can still return decryptedMessage (currently zero-filled via Buffer.alloc) as long as the signature verifies. Capture and validate the boolean result from crypto_box_open_easy and abort when it’s false.
Description
sodium_malloc allocates guarded memory pages via mmap, creating at least 3 virtual memory areas per call. The decrypt path was calling it for every inbound peer message to allocate the decryptedMessage buffer along with auth and verification key buffers.
Under normal P2P traffic, these mmap regions accumulated faster than V8's GC could reclaim them because the GC has no visibility into kernel VMA pressure. It only sees tiny JS heap wrappers. Eventually the process exceeded vm.max_map_count (65530), mmap returned null, and Node crashed on the ArrayBuffer assertion.
Fixed by switching all ephemeral allocations in the encrypt, decrypt, authenticate, and verify paths to Buffer.alloc. sodium_malloc is now only used for long-lived secret key material where guard pages and swap protection actually matter.
Type of Change
Checklist
Build & Tests
npm installcompletes without errorsnpm run buildcompletes without errorsnpm testpasses all testsCode Quality
Documentation
Security
OP_NET Node Specific
Testing
Consensus Impact
Related Issues
By submitting this PR, I confirm that my contribution is made under the terms of the project's license.