Skip to content

Commit 8344719

Browse files
mhdawsonmarco-ippolito
authored andcommitted
crypto: disable PKCS#1 padding for privateDecrypt
Refs: https://hackerone.com/bugs?subject=nodejs&report_id=2269177 Disable RSA_PKCS1_PADDING for crypto.privateDecrypt() in order to protect against the Marvin attack. Includes a security revert flag that can be used to restore support. Signed-off-by: Michael Dawson <midawson@redhat.com> PR-URL: nodejs-private/node-private#525 Refs: https://hackerone.com/bugs?subject=nodejs&report_id=2269177 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> CVE-ID: CVE-2023-46809
1 parent 6155a1f commit 8344719

File tree

4 files changed

+535
-14
lines changed

4 files changed

+535
-14
lines changed

src/crypto/crypto_cipher.cc

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include "node_buffer.h"
77
#include "node_internals.h"
88
#include "node_process-inl.h"
9+
#include "node_revert.h"
910
#include "v8.h"
1011

1112
namespace node {
@@ -1052,6 +1053,33 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo<Value>& args) {
10521053
uint32_t padding;
10531054
if (!args[offset + 1]->Uint32Value(env->context()).To(&padding)) return;
10541055

1056+
if (EVP_PKEY_cipher == EVP_PKEY_decrypt &&
1057+
operation == PublicKeyCipher::kPrivate && padding == RSA_PKCS1_PADDING &&
1058+
!IsReverted(SECURITY_REVERT_CVE_2023_46809)) {
1059+
EVPKeyCtxPointer ctx(EVP_PKEY_CTX_new(pkey.get(), nullptr));
1060+
CHECK(ctx);
1061+
1062+
if (EVP_PKEY_decrypt_init(ctx.get()) <= 0) {
1063+
return ThrowCryptoError(env, ERR_get_error());
1064+
}
1065+
1066+
int rsa_pkcs1_implicit_rejection =
1067+
EVP_PKEY_CTX_ctrl_str(ctx.get(), "rsa_pkcs1_implicit_rejection", "1");
1068+
// From the doc -2 means that the option is not supported.
1069+
// The default for the option is enabled and if it has been
1070+
// specifically disabled we want to respect that so we will
1071+
// not throw an error if the option is supported regardless
1072+
// of how it is set. The call to set the value
1073+
// will not affect what is used since a different context is
1074+
// used in the call if the option is supported
1075+
if (rsa_pkcs1_implicit_rejection <= 0) {
1076+
return THROW_ERR_INVALID_ARG_VALUE(
1077+
env,
1078+
"RSA_PKCS1_PADDING is no longer supported for private decryption,"
1079+
" this can be reverted with --security-revert=CVE-2023-46809");
1080+
}
1081+
}
1082+
10551083
const EVP_MD* digest = nullptr;
10561084
if (args[offset + 2]->IsString()) {
10571085
const Utf8Value oaep_str(env->isolate(), args[offset + 2]);

src/node_revert.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@
1515
**/
1616
namespace node {
1717

18-
#define SECURITY_REVERSIONS(XX) \
19-
// XX(CVE_2016_PEND, "CVE-2016-PEND", "Vulnerability Title")
18+
#define SECURITY_REVERSIONS(XX) \
19+
XX(CVE_2023_46809, "CVE-2023-46809", "Marvin attack on PKCS#1 padding")
2020

2121
enum reversion {
2222
#define V(code, ...) SECURITY_REVERT_##code,

0 commit comments

Comments
 (0)