Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions src/boringssl/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,30 +94,33 @@ pub unsafe fn ssl_ctx_setup(ctx: *mut boring::SSL_CTX) {
// into the process, including pthreads locks. Failing to meet these constraints
// may result in deadlocks, crashes, or memory corruption.

// `default_alloc` (mimalloc, or libc under `cfg(bun_asan)`): on ELF the weak
// hook symbols resolve to these definitions even when the BoringSSL build
// drops BORINGSSL_REQUIRE_MEMORY_HOOKS, so the allocator must match.
Comment thread
robobun marked this conversation as resolved.
#[unsafe(no_mangle)]
pub(crate) extern "C" fn OPENSSL_memory_alloc(size: usize) -> *mut c_void {
bun_alloc::mimalloc::mi_malloc(size)
bun_alloc::default_alloc::malloc(size)
}

// BoringSSL always expects memory to be zero'd
/// # Safety
/// `ptr` must be non-null and have been returned by `OPENSSL_memory_alloc`
/// (i.e. `mi_malloc`); BoringSSL guarantees both for this hook.
/// `ptr` must be non-null and returned by `OPENSSL_memory_alloc`; BoringSSL
/// guarantees both for this hook.
Comment thread
robobun marked this conversation as resolved.
#[unsafe(no_mangle)]
pub(crate) unsafe extern "C" fn OPENSSL_memory_free(ptr: *mut c_void) {
// SAFETY: BoringSSL guarantees ptr is non-null and was returned by
// OPENSSL_memory_alloc above (i.e. mi_malloc).
// OPENSSL_memory_alloc above.
unsafe {
let len = bun_alloc::usable_size(ptr.cast());
let len = bun_alloc::default_alloc::usable_size(ptr);
ptr::write_bytes(ptr.cast::<u8>(), 0, len);
bun_alloc::mimalloc::mi_free(ptr);
bun_alloc::default_alloc::free(ptr);
}
}

#[unsafe(no_mangle)]
pub(crate) extern "C" fn OPENSSL_memory_get_size(ptr: *const c_void) -> usize {
// ptr was returned by mi_malloc (or is null, which usable_size handles).
bun_alloc::usable_size(ptr.cast())
// SAFETY: ptr was returned by OPENSSL_memory_alloc (null-safe).
unsafe { bun_alloc::default_alloc::usable_size(ptr) }
}

pub use bun_sys::posix::INET6_ADDRSTRLEN;
Expand Down
10 changes: 1 addition & 9 deletions src/bun_alloc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,7 @@ pub mod default_alloc {
/// # Safety
/// `ptr` must be null or a live allocation from the default allocator.
#[inline]
#[cfg(any(debug_assertions, bun_asan))]
pub(crate) unsafe fn usable_size(ptr: *const c_void) -> usize {
pub unsafe fn usable_size(ptr: *const c_void) -> usize {
Comment thread
claude[bot] marked this conversation as resolved.
if ptr.is_null() {
return 0;
}
Expand Down Expand Up @@ -703,13 +702,6 @@ pub unsafe fn realloc_raw(
Ok(new_ptr.cast::<u8>())
}

/// `mi_usable_size` — actual allocated size for a mimalloc-owned ptr.
#[inline]
pub fn usable_size(ptr: *const u8) -> usize {
// SAFETY: `mi_usable_size` is null-safe (returns 0).
unsafe { mimalloc::mi_usable_size(ptr.cast()) }
}

// ──────────────────────────────────────────────────────────────────────────
// Symbols hoisted DOWN into T0 so higher tiers can re-import without cycles.
// ──────────────────────────────────────────────────────────────────────────
Expand Down
4 changes: 4 additions & 0 deletions src/jsc/bindings/node/crypto/CryptoDhJob.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ void DhJobCtx::runFromJS(JSGlobalObject* lexicalGlobalObject, JSValue callback)
VM& vm = lexicalGlobalObject->vm();
ThrowScope scope = DECLARE_THROW_SCOPE(vm);

// Drop before re-entering JS; see KeyPairJobCtx::runFromJS.
m_privateKey = nullptr;
m_publicKey = nullptr;

if (!m_result) {
// Same message as the synchronous path so callers observe identical errors either way.
JSObject* err = createError(lexicalGlobalObject, ErrorCode::ERR_CRYPTO_OPERATION_FAILED, "diffieHellman operation failed"_s);
Expand Down
6 changes: 6 additions & 0 deletions src/jsc/bindings/node/crypto/CryptoGenKeyPair.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ void KeyPairJobCtx::runFromJS(JSGlobalObject* lexicalGlobalObject, JSValue callb
if (scope.exception()) [[unlikely]] {
JSValue exceptionValue = scope.exception();
(void)scope.tryClearException();
m_keyObj = {};
exceptionCallback(exceptionValue);
return;
}
Expand All @@ -56,10 +57,15 @@ void KeyPairJobCtx::runFromJS(JSGlobalObject* lexicalGlobalObject, JSValue callb
if (scope.exception()) [[unlikely]] {
JSValue exceptionValue = scope.exception();
(void)scope.tryClearException();
m_keyObj = {};
exceptionCallback(exceptionValue);
return;
}

// Drop our ref before re-entering JS: the ctx is freed only after the
// callback returns, so process.exit() inside it would strand the EVP_PKEY.
Comment thread
robobun marked this conversation as resolved.
m_keyObj = {};

Bun__EventLoop__runCallback3(
lexicalGlobalObject,
JSValue::encode(callback),
Expand Down
3 changes: 3 additions & 0 deletions src/jsc/bindings/node/crypto/CryptoSignJob.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,9 @@ void SignJobCtx::runFromJS(JSGlobalObject* lexicalGlobalObject, JSValue callback
auto& vm = lexicalGlobalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);

// Drop before re-entering JS; see KeyPairJobCtx::runFromJS.
m_keyData = nullptr;

switch (m_mode) {
case Mode::Sign: {
if (!m_signResult) {
Expand Down
71 changes: 70 additions & 1 deletion test/js/node/crypto/crypto.key-objects.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
verify,
} from "crypto";
import fs from "fs";
import { isWindows } from "harness";
import { bunEnv, bunExe, isASAN, isWindows } from "harness";
import { createContext, runInContext, runInThisContext, Script } from "node:vm";
import path from "path";

Expand Down Expand Up @@ -1800,3 +1800,72 @@ test("ECDSA should work", async () => {
function randomProp() {
return "prop" + crypto.randomUUID().replace(/-/g, "");
}

// The generateKeyPair callback re-enters JS while the job ctx still owns a
// RefPtr<KeyObjectData>; process.exit() inside the callback never unwinds so
// that ref is never released and the EVP_PKEY survives past VM teardown. With
// OPENSSL_malloc routed through the default allocator (libc under ASAN) LSan
// reports it. The fix drops the ctx's ref before invoking the callback.
describe.skipIf(!isASAN)("generateKeyPair: process.exit() in the callback does not strand the EVP_PKEY", () => {
const lsanEnv = {
...bunEnv,
BUN_DESTRUCT_VM_ON_EXIT: "1",
ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "detect_leaks=1"].filter(Boolean).join(":"),
LSAN_OPTIONS: `print_suppressions=0:suppressions=${path.join(import.meta.dirname, "../../../leaksan.supp")}`,
};
// LSan symbolizes the leak stack through llvm-symbolizer before the child
// can exit, which is several seconds against the debug binary.
const timeout = 30_000;

test.concurrent(
"KeyObject output",
async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`require("crypto").generateKeyPair("rsa", { modulusLength: 512 }, (err, pub, priv) => {
if (err) { console.error(err); process.exit(2); }
if (pub.type !== "public" || priv.type !== "private") process.exit(3);
process.exit(0);
});`,
],
env: lsanEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).not.toContain("LeakSanitizer");
expect({ stdout, stderr, exitCode }).toEqual({ stdout: "", stderr: "", exitCode: 0 });
},
timeout,
);

test.concurrent(
"encrypted PEM output",
async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`require("crypto").generateKeyPair("rsa", {
modulusLength: 512,
publicKeyEncoding: { type: "spki", format: "pem" },
privateKeyEncoding: { type: "pkcs8", format: "pem", cipher: "aes-256-cbc", passphrase: "secret" },
}, (err, pub, priv) => {
if (err) { console.error(err); process.exit(2); }
if (typeof pub !== "string" || typeof priv !== "string") process.exit(3);
process.exit(0);
});`,
],
env: lsanEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).not.toContain("LeakSanitizer");
expect({ stdout, stderr, exitCode }).toEqual({ stdout: "", stderr: "", exitCode: 0 });
},
timeout,
);
});
Loading