Skip to content

Commit 7d0c17b

Browse files
RafaelGSSpanvatniessenaddaleax
committed
src: fix error handling on async crypto operations
Fixes: https://hackerone.com/reports/2817648 Co-Authored-By: Filip Skokan <[email protected]> Co-Authored-By: Tobias Nießen <[email protected]> Co-Authored-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> CVE-ID: CVE-2025-23166 PR-URL: nodejs-private/node-private#688
1 parent c5f1fcb commit 7d0c17b

20 files changed

+120
-89
lines changed

‎src/crypto/crypto_dh.cc

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -512,15 +512,15 @@ MaybeLocal<Value> DHBitsTraits::EncodeOutput(Environment* env,
512512
return out->ToArrayBuffer(env);
513513
}
514514

515-
bool DHBitsTraits::DeriveBits(
516-
Environment* env,
517-
const DHBitsConfig& params,
518-
ByteSource* out) {
515+
bool DHBitsTraits::DeriveBits(Environment* env,
516+
const DHBitsConfig& params,
517+
ByteSource* out,
518+
CryptoJobMode mode) {
519519
auto dp = DHPointer::stateless(params.private_key.GetAsymmetricKey(),
520520
params.public_key.GetAsymmetricKey());
521521
if (!dp) {
522-
bool can_throw =
523-
per_process::v8_initialized && Isolate::TryGetCurrent() != nullptr;
522+
bool can_throw = mode == CryptoJobMode::kCryptoJobSync;
523+
524524
if (can_throw) {
525525
unsigned long err = ERR_get_error(); // NOLINT(runtime/int)
526526
if (err) ThrowCryptoError(env, err, "diffieHellman failed");

‎src/crypto/crypto_dh.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,10 +103,10 @@ struct DHBitsTraits final {
103103
unsigned int offset,
104104
DHBitsConfig* params);
105105

106-
static bool DeriveBits(
107-
Environment* env,
108-
const DHBitsConfig& params,
109-
ByteSource* out_);
106+
static bool DeriveBits(Environment* env,
107+
const DHBitsConfig& params,
108+
ByteSource* out_,
109+
CryptoJobMode mode);
110110

111111
static v8::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
112112
const DHBitsConfig& params,

‎src/crypto/crypto_ec.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,8 @@ Maybe<void> ECDHBitsTraits::AdditionalConfig(
434434

435435
bool ECDHBitsTraits::DeriveBits(Environment* env,
436436
const ECDHBitsConfig& params,
437-
ByteSource* out) {
437+
ByteSource* out,
438+
CryptoJobMode mode) {
438439
size_t len = 0;
439440
const auto& m_privkey = params.private_.GetAsymmetricKey();
440441
const auto& m_pubkey = params.public_.GetAsymmetricKey();

‎src/crypto/crypto_ec.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,10 @@ struct ECDHBitsTraits final {
7777
unsigned int offset,
7878
ECDHBitsConfig* params);
7979

80-
static bool DeriveBits(
81-
Environment* env,
82-
const ECDHBitsConfig& params,
83-
ByteSource* out_);
80+
static bool DeriveBits(Environment* env,
81+
const ECDHBitsConfig& params,
82+
ByteSource* out_,
83+
CryptoJobMode mode);
8484

8585
static v8::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
8686
const ECDHBitsConfig& params,

‎src/crypto/crypto_hash.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -481,10 +481,10 @@ Maybe<void> HashTraits::AdditionalConfig(
481481
return JustVoid();
482482
}
483483

484-
bool HashTraits::DeriveBits(
485-
Environment* env,
486-
const HashConfig& params,
487-
ByteSource* out) {
484+
bool HashTraits::DeriveBits(Environment* env,
485+
const HashConfig& params,
486+
ByteSource* out,
487+
CryptoJobMode mode) {
488488
auto ctx = EVPMDCtxPointer::New();
489489

490490
if (!ctx.digestInit(params.digest) || !ctx.digestUpdate(params.in))

‎src/crypto/crypto_hash.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,10 @@ struct HashTraits final {
7070
unsigned int offset,
7171
HashConfig* params);
7272

73-
static bool DeriveBits(
74-
Environment* env,
75-
const HashConfig& params,
76-
ByteSource* out);
73+
static bool DeriveBits(Environment* env,
74+
const HashConfig& params,
75+
ByteSource* out,
76+
CryptoJobMode mode);
7777

7878
static v8::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
7979
const HashConfig& params,

‎src/crypto/crypto_hkdf.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,10 @@ Maybe<void> HKDFTraits::AdditionalConfig(
9797
return JustVoid();
9898
}
9999

100-
bool HKDFTraits::DeriveBits(
101-
Environment* env,
102-
const HKDFConfig& params,
103-
ByteSource* out) {
100+
bool HKDFTraits::DeriveBits(Environment* env,
101+
const HKDFConfig& params,
102+
ByteSource* out,
103+
CryptoJobMode mode) {
104104
auto dp = ncrypto::hkdf(params.digest,
105105
ncrypto::Buffer<const unsigned char>{
106106
.data = reinterpret_cast<const unsigned char*>(

‎src/crypto/crypto_hkdf.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,10 @@ struct HKDFTraits final {
4242
unsigned int offset,
4343
HKDFConfig* params);
4444

45-
static bool DeriveBits(
46-
Environment* env,
47-
const HKDFConfig& params,
48-
ByteSource* out);
45+
static bool DeriveBits(Environment* env,
46+
const HKDFConfig& params,
47+
ByteSource* out,
48+
CryptoJobMode mode);
4949

5050
static v8::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
5151
const HKDFConfig& params,

‎src/crypto/crypto_hmac.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -228,10 +228,10 @@ Maybe<void> HmacTraits::AdditionalConfig(
228228
return JustVoid();
229229
}
230230

231-
bool HmacTraits::DeriveBits(
232-
Environment* env,
233-
const HmacConfig& params,
234-
ByteSource* out) {
231+
bool HmacTraits::DeriveBits(Environment* env,
232+
const HmacConfig& params,
233+
ByteSource* out,
234+
CryptoJobMode mode) {
235235
auto ctx = HMACCtxPointer::New();
236236

237237
ncrypto::Buffer<const void> key_buf{

‎src/crypto/crypto_hmac.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,10 @@ struct HmacTraits final {
7373
unsigned int offset,
7474
HmacConfig* params);
7575

76-
static bool DeriveBits(
77-
Environment* env,
78-
const HmacConfig& params,
79-
ByteSource* out);
76+
static bool DeriveBits(Environment* env,
77+
const HmacConfig& params,
78+
ByteSource* out,
79+
CryptoJobMode mode);
8080

8181
static v8::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
8282
const HmacConfig& params,

‎src/crypto/crypto_pbkdf2.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,8 @@ Maybe<void> PBKDF2Traits::AdditionalConfig(
112112

113113
bool PBKDF2Traits::DeriveBits(Environment* env,
114114
const PBKDF2Config& params,
115-
ByteSource* out) {
115+
ByteSource* out,
116+
CryptoJobMode mode) {
116117
// Both pass and salt may be zero length here.
117118
auto dp = ncrypto::pbkdf2(params.digest,
118119
ncrypto::Buffer<const char>{

‎src/crypto/crypto_pbkdf2.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,10 @@ struct PBKDF2Traits final {
5555
unsigned int offset,
5656
PBKDF2Config* params);
5757

58-
static bool DeriveBits(
59-
Environment* env,
60-
const PBKDF2Config& params,
61-
ByteSource* out);
58+
static bool DeriveBits(Environment* env,
59+
const PBKDF2Config& params,
60+
ByteSource* out,
61+
CryptoJobMode mode);
6262

6363
static v8::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
6464
const PBKDF2Config& params,

‎src/crypto/crypto_random.cc

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,10 @@ Maybe<void> RandomBytesTraits::AdditionalConfig(
6565
return JustVoid();
6666
}
6767

68-
bool RandomBytesTraits::DeriveBits(
69-
Environment* env,
70-
const RandomBytesConfig& params,
71-
ByteSource* unused) {
68+
bool RandomBytesTraits::DeriveBits(Environment* env,
69+
const RandomBytesConfig& params,
70+
ByteSource* unused,
71+
CryptoJobMode mode) {
7272
return ncrypto::CSPRNG(params.buffer, params.size);
7373
}
7474

@@ -154,7 +154,8 @@ Maybe<void> RandomPrimeTraits::AdditionalConfig(
154154

155155
bool RandomPrimeTraits::DeriveBits(Environment* env,
156156
const RandomPrimeConfig& params,
157-
ByteSource* unused) {
157+
ByteSource* unused,
158+
CryptoJobMode mode) {
158159
return params.prime.generate(
159160
BignumPointer::PrimeConfig{
160161
.bits = params.bits,
@@ -190,10 +191,10 @@ Maybe<void> CheckPrimeTraits::AdditionalConfig(
190191
return JustVoid();
191192
}
192193

193-
bool CheckPrimeTraits::DeriveBits(
194-
Environment* env,
195-
const CheckPrimeConfig& params,
196-
ByteSource* out) {
194+
bool CheckPrimeTraits::DeriveBits(Environment* env,
195+
const CheckPrimeConfig& params,
196+
ByteSource* out,
197+
CryptoJobMode mode) {
197198
int ret = params.candidate.isPrime(params.checks, getPrimeCheckCallback(env));
198199
if (ret < 0) [[unlikely]]
199200
return false;

‎src/crypto/crypto_random.h

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,10 @@ struct RandomBytesTraits final {
3232
unsigned int offset,
3333
RandomBytesConfig* params);
3434

35-
static bool DeriveBits(
36-
Environment* env,
37-
const RandomBytesConfig& params,
38-
ByteSource* out_);
35+
static bool DeriveBits(Environment* env,
36+
const RandomBytesConfig& params,
37+
ByteSource* out_,
38+
CryptoJobMode mode);
3939

4040
static v8::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
4141
const RandomBytesConfig& params,
@@ -67,10 +67,10 @@ struct RandomPrimeTraits final {
6767
unsigned int offset,
6868
RandomPrimeConfig* params);
6969

70-
static bool DeriveBits(
71-
Environment* env,
72-
const RandomPrimeConfig& params,
73-
ByteSource* out_);
70+
static bool DeriveBits(Environment* env,
71+
const RandomPrimeConfig& params,
72+
ByteSource* out_,
73+
CryptoJobMode mode);
7474

7575
static v8::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
7676
const RandomPrimeConfig& params,
@@ -101,10 +101,10 @@ struct CheckPrimeTraits final {
101101
unsigned int offset,
102102
CheckPrimeConfig* params);
103103

104-
static bool DeriveBits(
105-
Environment* env,
106-
const CheckPrimeConfig& params,
107-
ByteSource* out);
104+
static bool DeriveBits(Environment* env,
105+
const CheckPrimeConfig& params,
106+
ByteSource* out,
107+
CryptoJobMode mode);
108108

109109
static v8::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
110110
const CheckPrimeConfig& params,

‎src/crypto/crypto_scrypt.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,10 @@ Maybe<void> ScryptTraits::AdditionalConfig(
113113
return JustVoid();
114114
}
115115

116-
bool ScryptTraits::DeriveBits(
117-
Environment* env,
118-
const ScryptConfig& params,
119-
ByteSource* out) {
116+
bool ScryptTraits::DeriveBits(Environment* env,
117+
const ScryptConfig& params,
118+
ByteSource* out,
119+
CryptoJobMode mode) {
120120
// If the params.length is zero-length, just return an empty buffer.
121121
// It's useless, yes, but allowed via the API.
122122
if (params.length == 0) {

‎src/crypto/crypto_scrypt.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,10 @@ struct ScryptTraits final {
5757
unsigned int offset,
5858
ScryptConfig* params);
5959

60-
static bool DeriveBits(
61-
Environment* env,
62-
const ScryptConfig& params,
63-
ByteSource* out);
60+
static bool DeriveBits(Environment* env,
61+
const ScryptConfig& params,
62+
ByteSource* out,
63+
CryptoJobMode mode);
6464

6565
static v8::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
6666
const ScryptConfig& params,

‎src/crypto/crypto_sig.cc

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -634,11 +634,11 @@ Maybe<void> SignTraits::AdditionalConfig(
634634
return JustVoid();
635635
}
636636

637-
bool SignTraits::DeriveBits(
638-
Environment* env,
639-
const SignConfiguration& params,
640-
ByteSource* out) {
641-
ClearErrorOnReturn clear_error_on_return;
637+
bool SignTraits::DeriveBits(Environment* env,
638+
const SignConfiguration& params,
639+
ByteSource* out,
640+
CryptoJobMode mode) {
641+
bool can_throw = mode == CryptoJobMode::kCryptoJobSync;
642642
auto context = EVPMDCtxPointer::New();
643643
if (!context) [[unlikely]]
644644
return false;
@@ -655,7 +655,7 @@ bool SignTraits::DeriveBits(
655655
})();
656656

657657
if (!ctx.has_value()) [[unlikely]] {
658-
crypto::CheckThrow(env, SignBase::Error::Init);
658+
if (can_throw) crypto::CheckThrow(env, SignBase::Error::Init);
659659
return false;
660660
}
661661

@@ -669,7 +669,7 @@ bool SignTraits::DeriveBits(
669669
: std::nullopt;
670670

671671
if (!ApplyRSAOptions(key, *ctx, padding, salt_length)) {
672-
crypto::CheckThrow(env, SignBase::Error::PrivateKey);
672+
if (can_throw) crypto::CheckThrow(env, SignBase::Error::PrivateKey);
673673
return false;
674674
}
675675

@@ -678,15 +678,15 @@ bool SignTraits::DeriveBits(
678678
if (key.isOneShotVariant()) {
679679
auto data = context.signOneShot(params.data);
680680
if (!data) [[unlikely]] {
681-
crypto::CheckThrow(env, SignBase::Error::PrivateKey);
681+
if (can_throw) crypto::CheckThrow(env, SignBase::Error::PrivateKey);
682682
return false;
683683
}
684684
DCHECK(!data.isSecure());
685685
*out = ByteSource::Allocated(data.release());
686686
} else {
687687
auto data = context.sign(params.data);
688688
if (!data) [[unlikely]] {
689-
crypto::CheckThrow(env, SignBase::Error::PrivateKey);
689+
if (can_throw) crypto::CheckThrow(env, SignBase::Error::PrivateKey);
690690
return false;
691691
}
692692
DCHECK(!data.isSecure());

‎src/crypto/crypto_sig.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,10 +137,10 @@ struct SignTraits final {
137137
unsigned int offset,
138138
SignConfiguration* params);
139139

140-
static bool DeriveBits(
141-
Environment* env,
142-
const SignConfiguration& params,
143-
ByteSource* out);
140+
static bool DeriveBits(Environment* env,
141+
const SignConfiguration& params,
142+
ByteSource* out,
143+
CryptoJobMode mode);
144144

145145
static v8::MaybeLocal<v8::Value> EncodeOutput(Environment* env,
146146
const SignConfiguration& params,

‎src/crypto/crypto_util.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -410,9 +410,11 @@ class DeriveBitsJob final : public CryptoJob<DeriveBitsTraits> {
410410
std::move(params)) {}
411411

412412
void DoThreadPoolWork() override {
413-
if (!DeriveBitsTraits::DeriveBits(
414-
AsyncWrap::env(),
415-
*CryptoJob<DeriveBitsTraits>::params(), &out_)) {
413+
ncrypto::ClearErrorOnReturn clear_error_on_return;
414+
if (!DeriveBitsTraits::DeriveBits(AsyncWrap::env(),
415+
*CryptoJob<DeriveBitsTraits>::params(),
416+
&out_,
417+
this->mode())) {
416418
CryptoErrorStore* errors = CryptoJob<DeriveBitsTraits>::errors();
417419
errors->Capture();
418420
if (errors->Empty())

0 commit comments

Comments
 (0)