Repository navigation
crypto: cipher update process crash with input of max int32 length #45757
Description
Activity
cc @nodejs/crypto
- changed the title
[-]crypto cause process crash with magic data length[/-][+]crypto: cipher update process crash with input of max int32 length[/+]on Dec 6, 2022 # # Fatal error in , line 0 # Check failed: byte_length <= i::JSArrayBuffer::kMaxByteLength. # # # #FailureMessage Object: 0x16d365b68 1: 0x102baf630 node::NodePlatform::GetStackTracePrinter()::$_3::__invoke() 2: 0x103ac31dc V8_Fatal(char const*, ...) 3: 0x102cc068c v8::ArrayBuffer::NewBackingStore(v8::Isolate*, unsigned long) 4: 0x102c3ff8c node::crypto::CipherBase::Update(char const*, unsigned long, std::__1::unique_ptr<v8::BackingStore, std::__1::default_delete<v8::BackingStore> >*) 5: 0x102c406d0 node::crypto::CipherBase::Update(v8::FunctionCallbackInfo<v8::Value> const&)::$_0::__invoke(node::crypto::CipherBase*, v8::FunctionCallbackInfo<v8::Value> const&, char const*, unsigned long) 6: 0x102c3864c void node::crypto::Decode<node::crypto::Verify>(v8::FunctionCallbackInfo<v8::Value> const&, void (*)(node::crypto::Verify*, v8::FunctionCallbackInfo<v8::Value> const&, char const*, unsigned long)) 7: 0x102d0920c v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) 8: 0x102d08d08 v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) 9: 0x102d08534 v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) 10: 0x1034f918c Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit 11: 0x103484198 Builtins_InterpreterEntryTrampoline 12: 0x103484198 Builtins_InterpreterEntryTrampoline 13: 0x103484198 Builtins_InterpreterEntryTrampoline 14: 0x103484198 Builtins_InterpreterEntryTrampoline 15: 0x103484198 Builtins_InterpreterEntryTrampoline 16: 0x103484198 Builtins_InterpreterEntryTrampoline 17: 0x103484198 Builtins_InterpreterEntryTrampoline 18: 0x103484198 Builtins_InterpreterEntryTrampoline 19: 0x1034824d0 Builtins_JSEntryTrampoline 20: 0x103482164 Builtins_JSEntry 21: 0x102dc4eac v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) 22: 0x102dc43e0 v8::internal::Execution::Call(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*) 23: 0x102cb4914 v8::Function::Call(v8::Local<v8::Context>, v8::Local<v8::Value>, int, v8::Local<v8::Value>*) 24: 0x102b1d204 node::ExecuteBootstrapper(node::Environment*, char const*, std::__1::vector<v8::Local<v8::Value>, std::__1::allocator<v8::Local<v8::Value> > >*) 25: 0x102b1e074 node::StartExecution(node::Environment*, std::__1::function<v8::MaybeLocal<v8::Value> (node::StartExecutionCallbackInfo const&)>) 26: 0x102aa0420 node::LoadEnvironment(node::Environment*, std::__1::function<v8::MaybeLocal<v8::Value> (node::StartExecutionCallbackInfo const&)>) 27: 0x102b8d0f8 node::NodeMainInstance::Run() 28: 0x102b20ec8 node::LoadSnapshotDataAndRun(node::SnapshotData const**, node::InitializationResult const*) 29: 0x102b21190 node::Start(int, char**) 30: 0x18e6efe50 start [/usr/lib/dyld] [1] 17989 trace trap node some.cjsI'll work on it. If need help can I bother you @panva ?
It's a signed integer overflow, caused by openssl using ints for sizes and node mixing
intandsize_t(and not being diligent enough about overflow checking.)This particular issue isn't that hard to fix but it's probably just one of many similar bugs lurking in src/crypto.
Here is a quick fix:
diff --git a/src/crypto/crypto_cipher.cc b/src/crypto/crypto_cipher.cc index b907e9e9cdc..2259e28bec8 100644 --- a/src/crypto/crypto_cipher.cc +++ b/src/crypto/crypto_cipher.cc @@ -803,7 +803,11 @@ CipherBase::UpdateResult CipherBase::Update( if (kind_ == kDecipher && IsAuthenticatedMode()) CHECK(MaybePassAuthTagToOpenSSL()); - int buf_len = len + EVP_CIPHER_CTX_block_size(ctx_.get()); + const int block_size = EVP_CIPHER_CTX_block_size(ctx_.get()); + CHECK_GT(block_size, 0); + if (len + block_size > INT_MAX) return kErrorState; + + int buf_len = len + block_size; // For key wrapping algorithms, get output size by calling // EVP_CipherUpdate() with null output. if (kind_ == kCipher && mode == EVP_CIPH_WRAP_MODE &&
Note:
len + block_sizeis not UB becauseblock_sizegets converted tosize_tsincelenis asize_t.Reacted by Marco Ippolito@marco-ippolito absolutely, go for it.
- added a commit that references this issue
on Dec 9, 2022 - added 2 commits that reference this issue
on Dec 12, 2022
Version
v18.12.0
Platform
No response
Subsystem
crypto
What steps will reproduce the bug?
How often does it reproduce? Is there a required condition?
Every time.
What is the expected behavior?
Throw a normal catchable error like
size>=2**31do, or work right likesize<=2**31.What do you see instead?
Only
size===2**31-1will cause an uncatchable process crash.Additional information
No response