From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Tue, 21 Jun 2022 10:04:21 -0700 Subject: support V8 sandboxed pointers This refactors several allocators to allocate within the V8 memory cage, allowing them to be compatible with the V8_SANDBOXED_POINTERS feature. diff --git a/src/api/environment.cc b/src/api/environment.cc index e044f10284f31f1862b18be752a04b3bd5d53401..89ce587cac4506c4218a9316fe0b68070a7a8504 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -102,6 +102,14 @@ MaybeLocal PrepareStackTraceCallback(Local context, return result; } +NodeArrayBufferAllocator::NodeArrayBufferAllocator() { + zero_fill_field_ = static_cast(allocator_->Allocate(sizeof(*zero_fill_field_))); +} + +NodeArrayBufferAllocator::~NodeArrayBufferAllocator() { + allocator_->Free(zero_fill_field_, sizeof(*zero_fill_field_)); +} + void* NodeArrayBufferAllocator::Allocate(size_t size) { void* ret; if (zero_fill_field_ || per_process::cli_options->zero_fill_all_buffers) diff --git a/src/crypto/crypto_dh.cc b/src/crypto/crypto_dh.cc index f973941b3b9ea954f35f2ea135f8ee3d77b98958..743c63ff7e3f526829919a8f2de7ebd625a93fbc 100644 --- a/src/crypto/crypto_dh.cc +++ b/src/crypto/crypto_dh.cc @@ -51,6 +51,25 @@ void DiffieHellman::MemoryInfo(MemoryTracker* tracker) const { namespace { MaybeLocal DataPointerToBuffer(Environment* env, ncrypto::DataPointer&& data) { +#if defined(V8_ENABLE_SANDBOX) + std::unique_ptr backing; + if (data.size() > 0) { + std::unique_ptr allocator(ArrayBuffer::Allocator::NewDefaultAllocator()); + void* v8_data = allocator->Allocate(data.size()); + CHECK(v8_data); + memcpy(v8_data, data.get(), data.size()); + backing = ArrayBuffer::NewBackingStore( + v8_data, + data.size(), + [](void* data, size_t length, void*) { + std::unique_ptr allocator(ArrayBuffer::Allocator::NewDefaultAllocator()); + allocator->Free(data, length); + }, nullptr); + } else { + NoArrayBufferZeroFillScope no_zero_fill_scope(env->isolate_data()); + backing = v8::ArrayBuffer::NewBackingStore(env->isolate(), data.size()); + } +#else auto backing = ArrayBuffer::NewBackingStore( data.get(), data.size(), @@ -59,6 +78,7 @@ MaybeLocal DataPointerToBuffer(Environment* env, }, nullptr); data.release(); +#endif auto ab = ArrayBuffer::New(env->isolate(), std::move(backing)); return Buffer::New(env, ab, 0, ab->ByteLength()).FromMaybe(Local()); diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc index 8a6a36a3c31532ed585c287ba8cee14026d315b4..3d449b5853f359d63e1b88671a857bf9152ff6af 100644 --- a/src/crypto/crypto_util.cc +++ b/src/crypto/crypto_util.cc @@ -326,10 +326,35 @@ ByteSource& ByteSource::operator=(ByteSource&& other) noexcept { return *this; } -std::unique_ptr ByteSource::ReleaseToBackingStore() { +std::unique_ptr ByteSource::ReleaseToBackingStore(Environment* env) { // It's ok for allocated_data_ to be nullptr but // only if size_ is zero. CHECK_IMPLIES(size_ > 0, allocated_data_ != nullptr); +#if defined(V8_ENABLE_SANDBOX) + // When V8 sandboxed pointers are enabled, we have to copy into the memory + // cage. We still want to ensure we erase the data on free though, so + // provide a custom deleter that calls OPENSSL_cleanse. + if (!size()) + return ArrayBuffer::NewBackingStore(env->isolate(), 0); + std::unique_ptr allocator(ArrayBuffer::Allocator::NewDefaultAllocator()); + void* v8_data = allocator->Allocate(size()); + CHECK(v8_data); + memcpy(v8_data, allocated_data_, size()); + OPENSSL_clear_free(allocated_data_, size()); + std::unique_ptr ptr = ArrayBuffer::NewBackingStore( + v8_data, + size(), + [](void* data, size_t length, void*) { + OPENSSL_cleanse(data, length); + std::unique_ptr allocator(ArrayBuffer::Allocator::NewDefaultAllocator()); + allocator->Free(data, length); + }, nullptr); + CHECK(ptr); + allocated_data_ = nullptr; + data_ = nullptr; + size_ = 0; + return ptr; +#else std::unique_ptr ptr = ArrayBuffer::NewBackingStore( allocated_data_, size(), @@ -341,10 +366,11 @@ std::unique_ptr ByteSource::ReleaseToBackingStore() { data_ = nullptr; size_ = 0; return ptr; +#endif // defined(V8_ENABLE_SANDBOX) } Local ByteSource::ToArrayBuffer(Environment* env) { - std::unique_ptr store = ReleaseToBackingStore(); + std::unique_ptr store = ReleaseToBackingStore(env); return ArrayBuffer::New(env->isolate(), std::move(store)); } @@ -641,6 +667,16 @@ namespace { // in which case this has the same semantics as // using OPENSSL_malloc. However, if the secure heap is // initialized, SecureBuffer will automatically use it. +#if defined(V8_ENABLE_SANDBOX) +// When V8 sandboxed pointers are enabled, the secure heap cannot be used as +// all ArrayBuffers must be allocated inside the V8 memory cage. +void SecureBuffer(const FunctionCallbackInfo& args) { + CHECK(args[0]->IsUint32()); + uint32_t len = args[0].As()->Value(); + Local buffer = ArrayBuffer::New(args.GetIsolate(), len); + args.GetReturnValue().Set(Uint8Array::New(buffer, 0, len)); +} +#else void SecureBuffer(const FunctionCallbackInfo& args) { CHECK(args[0]->IsUint32()); Environment* env = Environment::GetCurrent(args); @@ -662,6 +698,7 @@ void SecureBuffer(const FunctionCallbackInfo& args) { Local buffer = ArrayBuffer::New(env->isolate(), store); args.GetReturnValue().Set(Uint8Array::New(buffer, 0, len)); } +#endif // defined(V8_ENABLE_SANDBOX) void SecureHeapUsed(const FunctionCallbackInfo& args) { #ifndef OPENSSL_IS_BORINGSSL diff --git a/src/crypto/crypto_util.h b/src/crypto/crypto_util.h index 5c717c6fdb0fc453fa6c0061077300926af31ed5..b5fbe8e964943ab6f3842b27638f20ff64a1c0c4 100644 --- a/src/crypto/crypto_util.h +++ b/src/crypto/crypto_util.h @@ -268,7 +268,7 @@ class ByteSource { // Creates a v8::BackingStore that takes over responsibility for // any allocated data. The ByteSource will be reset with size = 0 // after being called. - std::unique_ptr ReleaseToBackingStore(); + std::unique_ptr ReleaseToBackingStore(Environment* env); v8::Local ToArrayBuffer(Environment* env); diff --git a/src/crypto/crypto_x509.cc b/src/crypto/crypto_x509.cc index 9b9bb7be9a8daca98a2635bf13cb6d1d561ea5fb..81afe2b5f7398f0c20b340648ca75022470be544 100644 --- a/src/crypto/crypto_x509.cc +++ b/src/crypto/crypto_x509.cc @@ -175,6 +175,19 @@ MaybeLocal ToV8Value(Local context, const BIOPointer& bio) { MaybeLocal ToBuffer(Environment* env, BIOPointer* bio) { if (bio == nullptr || !*bio) return {}; BUF_MEM* mem = *bio; +#if defined(V8_ENABLE_SANDBOX) + std::unique_ptr allocator(ArrayBuffer::Allocator::NewDefaultAllocator()); + void* v8_data = allocator->Allocate(mem->length); + CHECK(v8_data); + memcpy(v8_data, mem->data, mem->length); + std::unique_ptr backing = ArrayBuffer::NewBackingStore( + v8_data, + mem->length, + [](void* data, size_t length, void*) { + std::unique_ptr allocator(ArrayBuffer::Allocator::NewDefaultAllocator()); + allocator->Free(data, length); + }, nullptr); +#else auto backing = ArrayBuffer::NewBackingStore( mem->data, mem->length, @@ -182,6 +195,8 @@ MaybeLocal ToBuffer(Environment* env, BIOPointer* bio) { BIOPointer free_me(static_cast(data)); }, bio->release()); +#endif + auto ab = ArrayBuffer::New(env->isolate(), std::move(backing)); Local ret; if (!Buffer::New(env, ab, 0, ab->ByteLength()).ToLocal(&ret)) return {}; diff --git a/src/node_i18n.cc b/src/node_i18n.cc index 0bcf10a0b35accb8d6d5fe9891d4f52b27d40346..606c2021242e6967ea4195af3e2493a7d5745dae 100644 --- a/src/node_i18n.cc +++ b/src/node_i18n.cc @@ -104,7 +104,7 @@ namespace { template MaybeLocal ToBufferEndian(Environment* env, MaybeStackBuffer* buf) { - MaybeLocal ret = Buffer::New(env, buf); + MaybeLocal ret = Buffer::Copy(env, reinterpret_cast(buf->out()), buf->length() * sizeof(T)); if (ret.IsEmpty()) return ret; @@ -181,7 +181,7 @@ MaybeLocal TranscodeLatin1ToUcs2(Environment* env, return {}; } - return Buffer::New(env, &destbuf); + return Buffer::Copy(env, reinterpret_cast(destbuf.out()), destbuf.length() * sizeof(UChar)); } MaybeLocal TranscodeFromUcs2(Environment* env, @@ -226,7 +226,7 @@ MaybeLocal TranscodeUcs2FromUtf8(Environment* env, return {}; } - return Buffer::New(env, &destbuf); + return Buffer::Copy(env, reinterpret_cast(destbuf.out()), destbuf.length() * sizeof(UChar)); } MaybeLocal TranscodeUtf8FromUcs2(Environment* env, @@ -250,7 +250,7 @@ MaybeLocal TranscodeUtf8FromUcs2(Environment* env, return {}; } - return Buffer::New(env, &destbuf); + return Buffer::Copy(env, reinterpret_cast(destbuf.out()), destbuf.length() * sizeof(char)); } constexpr const char* EncodingName(const enum encoding encoding) { diff --git a/src/node_internals.h b/src/node_internals.h index 000ba16303740d7e48dcaf7b7c2e16fd750ac599..6396dc8f1a9db806ca4a4b547914680fcbaed9a1 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -117,7 +117,9 @@ v8::Maybe InitializePrimordials(v8::Local context); class NodeArrayBufferAllocator : public ArrayBufferAllocator { public: - inline uint32_t* zero_fill_field() { return &zero_fill_field_; } + NodeArrayBufferAllocator(); + ~NodeArrayBufferAllocator() override; + inline uint32_t* zero_fill_field() { return zero_fill_field_; } void* Allocate(size_t size) override; // Defined in src/node.cc void* AllocateUninitialized(size_t size) override; @@ -135,7 +137,7 @@ class NodeArrayBufferAllocator : public ArrayBufferAllocator { } private: - uint32_t zero_fill_field_ = 1; // Boolean but exposed as uint32 to JS land. + uint32_t* zero_fill_field_ = nullptr; // Boolean but exposed as uint32 to JS land. std::atomic total_mem_usage_ {0}; // Delegate to V8's allocator for compatibility with the V8 memory cage. diff --git a/src/node_serdes.cc b/src/node_serdes.cc index 7a70997bc024efa4f3ff4cabe30d5e88dcc7bc78..6552af3ed0acede41c1b16ef77eb359dc54f088a 100644 --- a/src/node_serdes.cc +++ b/src/node_serdes.cc @@ -29,6 +29,26 @@ using v8::ValueSerializer; namespace serdes { +v8::ArrayBuffer::Allocator* GetAllocator() { + static v8::ArrayBuffer::Allocator* allocator = v8::ArrayBuffer::Allocator::NewDefaultAllocator(); + return allocator; +} + +void* Reallocate(void* data, size_t old_length, + size_t new_length) { + if (old_length == new_length) return data; + uint8_t* new_data = + reinterpret_cast(GetAllocator()->AllocateUninitialized(new_length)); + if (new_data == nullptr) return nullptr; + size_t bytes_to_copy = std::min(old_length, new_length); + memcpy(new_data, data, bytes_to_copy); + if (new_length > bytes_to_copy) { + memset(new_data + bytes_to_copy, 0, new_length - bytes_to_copy); + } + GetAllocator()->Free(data, old_length); + return new_data; +} + class SerializerContext : public BaseObject, public ValueSerializer::Delegate { public: @@ -37,10 +57,15 @@ class SerializerContext : public BaseObject, ~SerializerContext() override = default; + // v8::ValueSerializer::Delegate void ThrowDataCloneError(Local message) override; Maybe WriteHostObject(Isolate* isolate, Local object) override; Maybe GetSharedArrayBufferId( Isolate* isolate, Local shared_array_buffer) override; + void* ReallocateBufferMemory(void* old_buffer, + size_t old_length, + size_t* new_length) override; + void FreeBufferMemory(void* buffer) override; static void SetTreatArrayBufferViewsAsHostObjects( const FunctionCallbackInfo& args); @@ -61,6 +86,7 @@ class SerializerContext : public BaseObject, private: ValueSerializer serializer_; + size_t last_length_ = 0; }; class DeserializerContext : public BaseObject, @@ -144,6 +170,24 @@ Maybe SerializerContext::GetSharedArrayBufferId( return id.ToLocalChecked()->Uint32Value(env()->context()); } +void* SerializerContext::ReallocateBufferMemory(void* old_buffer, + size_t requested_size, + size_t* new_length) { + *new_length = std::max(static_cast(4096), requested_size); + if (old_buffer) { + void* ret = Reallocate(old_buffer, last_length_, *new_length); + last_length_ = *new_length; + return ret; + } else { + last_length_ = *new_length; + return GetAllocator()->Allocate(*new_length); + } +} + +void SerializerContext::FreeBufferMemory(void* buffer) { + GetAllocator()->Free(buffer, last_length_); +} + Maybe SerializerContext::WriteHostObject(Isolate* isolate, Local input) { MaybeLocal ret; @@ -209,9 +253,14 @@ void SerializerContext::ReleaseBuffer(const FunctionCallbackInfo& args) { // Note: Both ValueSerializer and this Buffer::New() variant use malloc() // as the underlying allocator. std::pair ret = ctx->serializer_.Release(); - auto buf = Buffer::New(ctx->env(), - reinterpret_cast(ret.first), - ret.second); + std::unique_ptr bs = + v8::ArrayBuffer::NewBackingStore(reinterpret_cast(ret.first), ret.second, + [](void* data, size_t length, void* deleter_data) { + if (data) GetAllocator()->Free(reinterpret_cast(data), length); + }, nullptr); + Local ab = v8::ArrayBuffer::New(ctx->env()->isolate(), std::move(bs)); + + auto buf = Buffer::New(ctx->env(), ab, 0, ret.second); if (!buf.IsEmpty()) { args.GetReturnValue().Set(buf.ToLocalChecked()); diff --git a/src/node_trace_events.cc b/src/node_trace_events.cc index 9787b14352753c5e0f8dc2b90093680e7cd10f1a..31af9e62396368af1b81f8841a705fd313df2b9f 100644 --- a/src/node_trace_events.cc +++ b/src/node_trace_events.cc @@ -132,12 +132,28 @@ static void GetCategoryEnabledBuffer(const FunctionCallbackInfo& args) { const uint8_t* enabled_pointer = TRACE_EVENT_API_GET_CATEGORY_GROUP_ENABLED(category_name.out()); uint8_t* enabled_pointer_cast = const_cast(enabled_pointer); + uint8_t size = sizeof(*enabled_pointer_cast); +#if defined(V8_ENABLE_SANDBOX) + std::unique_ptr allocator(ArrayBuffer::Allocator::NewDefaultAllocator()); + void* v8_data = allocator->Allocate(size); + CHECK(v8_data); + memcpy(v8_data, enabled_pointer_cast, size); + std::unique_ptr bs = ArrayBuffer::NewBackingStore( + v8_data, + size, + [](void* data, size_t length, void*) { + std::unique_ptr allocator(ArrayBuffer::Allocator::NewDefaultAllocator()); + allocator->Free(data, length); + }, nullptr); +#else std::unique_ptr bs = ArrayBuffer::NewBackingStore( enabled_pointer_cast, - sizeof(*enabled_pointer_cast), + size, [](void*, size_t, void*) {}, nullptr); +#endif + auto ab = ArrayBuffer::New(isolate, std::move(bs)); v8::Local u8 = v8::Uint8Array::New(ab, 0, 1); diff --git a/test/parallel/test-buffer-tostring-range.js b/test/parallel/test-buffer-tostring-range.js index 73fec107a36c3db4af6f492137d0ca174f2d0547..a1153ec381f7b12a1640b611073f6997e1ec5696 100644 --- a/test/parallel/test-buffer-tostring-range.js +++ b/test/parallel/test-buffer-tostring-range.js @@ -102,8 +102,8 @@ assert.throws(() => { // Must not throw when start and end are within kMaxLength // Cannot test on 32bit machine as we are testing the case // when start and end are above the threshold -if (!common.openSSLIsBoringSSL) { +/* const threshold = 0xFFFFFFFF; const largeBuffer = Buffer.alloc(threshold + 20); largeBuffer.toString('utf8', threshold, threshold + 20); -} +*/