From 0bea17548c121f7094290a9edd7c12dc129f8e57 Mon Sep 17 00:00:00 2001 From: ndossche Date: Mon, 16 Feb 2026 13:37:47 +0100 Subject: [PATCH] fix: use proper return value check for EVP_CIPHER_CTX_ctrl() This function can theoretically return -1, which would then be converted to a truthy value. If this happens, then this can cause issues at the use sites. E.g. for the test-crypto-cipheriv-decipheriv test in Node this can cause a buffer overflow when we test with an injected error: ``` ==714700==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x502000032b98 READ of size 12 at 0x502000032b98 thread T0 #0 0x558a7790bb97 in memcpy (/work/node/out/Debug/node+0x1b0bb97) #1 0x7efe5386f90b (/lib/x86_64-linux-gnu/libcrypto.so.3+0x45f90b) #2 0x7efe536312c2 in EVP_CipherInit_ex (/lib/x86_64-linux-gnu/libcrypto.so.3+0x2212c2) #3 0x558a7d3e7785 in ncrypto::CipherCtxPointer::init /work/node/out/../deps/ncrypto/ncrypto.cc:3328:10 #4 0x558a78512a1b in node::crypto::CipherBase::CommonInit /work/node/out/../src/crypto/crypto_cipher.cc:366:13 #5 0x558a785125dd in node::crypto::CipherBase::InitIv /work/node/out/../src/crypto/crypto_cipher.cc:409:3 #6 0x558a7850f5f4 in node::crypto::CipherBase::New /work/node/out/../src/crypto/crypto_cipher.cc:328:11 #7 0x558a788ee605 in v8::internal::FunctionCallbackArguments::CallOrConstruct /work/node/out/../deps/v8/src/api/api-arguments-inl.h:93:3 #8 0x558a788ec3ba in v8::internal::MaybeHandle v8::internal::(anonymous namespace)::HandleApiCallHelper /work/node/out/../deps/v8/src/builtins/builtins-api.cc:104:16 #9 0x558a788e91fc in v8::internal::Builtin_Impl_HandleApiConstruct /work/node/out/../deps/v8/src/builtins/builtins-api.cc:135:3 #10 0x558a788e91fc in v8::internal::Builtin_HandleApiConstruct /work/node/out/../deps/v8/src/builtins/builtins-api.cc:126:1 #11 0x7efe31a951b5 () ``` --- src/ncrypto.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/ncrypto.cpp b/src/ncrypto.cpp index 50bd9c0..2d76f25 100644 --- a/src/ncrypto.cpp +++ b/src/ncrypto.cpp @@ -3403,19 +3403,21 @@ bool CipherCtxPointer::setKeyLength(size_t length) { bool CipherCtxPointer::setIvLength(size_t length) { if (!ctx_) return false; return EVP_CIPHER_CTX_ctrl( - ctx_.get(), EVP_CTRL_AEAD_SET_IVLEN, length, nullptr); + ctx_.get(), EVP_CTRL_AEAD_SET_IVLEN, length, nullptr) > 0; } bool CipherCtxPointer::setAeadTag(const Buffer& tag) { if (!ctx_) return false; - return EVP_CIPHER_CTX_ctrl( - ctx_.get(), EVP_CTRL_AEAD_SET_TAG, tag.len, const_cast(tag.data)); + return EVP_CIPHER_CTX_ctrl(ctx_.get(), + EVP_CTRL_AEAD_SET_TAG, + tag.len, + const_cast(tag.data)) > 0; } bool CipherCtxPointer::setAeadTagLength(size_t length) { if (!ctx_) return false; return EVP_CIPHER_CTX_ctrl( - ctx_.get(), EVP_CTRL_AEAD_SET_TAG, length, nullptr); + ctx_.get(), EVP_CTRL_AEAD_SET_TAG, length, nullptr) > 0; } bool CipherCtxPointer::setPadding(bool padding) { @@ -3485,7 +3487,7 @@ bool CipherCtxPointer::update(const Buffer& in, bool CipherCtxPointer::getAeadTag(size_t len, unsigned char* out) { if (!ctx_) return false; - return EVP_CIPHER_CTX_ctrl(ctx_.get(), EVP_CTRL_AEAD_GET_TAG, len, out); + return EVP_CIPHER_CTX_ctrl(ctx_.get(), EVP_CTRL_AEAD_GET_TAG, len, out) > 0; } // ============================================================================