Skip to content

Commit 69140bc

Browse files
committed
crypto: do not abort when setting throws
PR-URL: #27157 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 3ef1512 commit 69140bc

File tree

2 files changed

+69
-17
lines changed

2 files changed

+69
-17
lines changed

src/node_crypto.cc

+22-16
Original file line numberDiff line numberDiff line change
@@ -226,9 +226,9 @@ static int NoPasswordCallback(char* buf, int size, int rwflag, void* u) {
226226

227227
// namespace node::crypto::error
228228
namespace error {
229-
void Decorate(Environment* env, Local<Object> obj,
229+
Maybe<bool> Decorate(Environment* env, Local<Object> obj,
230230
unsigned long err) { // NOLINT(runtime/int)
231-
if (err == 0) return; // No decoration possible.
231+
if (err == 0) return Just(true); // No decoration necessary.
232232

233233
const char* ls = ERR_lib_error_string(err);
234234
const char* fs = ERR_func_error_string(err);
@@ -240,19 +240,19 @@ void Decorate(Environment* env, Local<Object> obj,
240240
if (ls != nullptr) {
241241
if (obj->Set(context, env->library_string(),
242242
OneByteString(isolate, ls)).IsNothing()) {
243-
return;
243+
return Nothing<bool>();
244244
}
245245
}
246246
if (fs != nullptr) {
247247
if (obj->Set(context, env->function_string(),
248248
OneByteString(isolate, fs)).IsNothing()) {
249-
return;
249+
return Nothing<bool>();
250250
}
251251
}
252252
if (rs != nullptr) {
253253
if (obj->Set(context, env->reason_string(),
254254
OneByteString(isolate, rs)).IsNothing()) {
255-
return;
255+
return Nothing<bool>();
256256
}
257257

258258
// SSL has no API to recover the error name from the number, so we
@@ -325,8 +325,10 @@ void Decorate(Environment* env, Local<Object> obj,
325325
if (obj->Set(env->isolate()->GetCurrentContext(),
326326
env->code_string(),
327327
OneByteString(env->isolate(), code)).IsNothing())
328-
return;
328+
return Nothing<bool>();
329329
}
330+
331+
return Just(true);
330332
}
331333
} // namespace error
332334

@@ -342,7 +344,7 @@ struct CryptoErrorVector : public std::vector<std::string> {
342344
std::reverse(begin(), end());
343345
}
344346

345-
inline Local<Value> ToException(
347+
inline MaybeLocal<Value> ToException(
346348
Environment* env,
347349
Local<String> exception_string = Local<String>()) const {
348350
if (exception_string.IsEmpty()) {
@@ -364,10 +366,11 @@ struct CryptoErrorVector : public std::vector<std::string> {
364366
if (!empty()) {
365367
CHECK(exception_v->IsObject());
366368
Local<Object> exception = exception_v.As<Object>();
367-
exception->Set(env->context(),
369+
Maybe<bool> ok = exception->Set(env->context(),
368370
env->openssl_error_stack(),
369-
ToV8Value(env->context(), *this).ToLocalChecked())
370-
.Check();
371+
ToV8Value(env->context(), *this).ToLocalChecked());
372+
if (ok.IsNothing())
373+
return MaybeLocal<Value>();
371374
}
372375

373376
return exception_v;
@@ -386,16 +389,19 @@ void ThrowCryptoError(Environment* env,
386389
message = message_buffer;
387390
}
388391
HandleScope scope(env->isolate());
389-
auto exception_string =
392+
Local<String> exception_string =
390393
String::NewFromUtf8(env->isolate(), message, NewStringType::kNormal)
391394
.ToLocalChecked();
392395
CryptoErrorVector errors;
393396
errors.Capture();
394-
Local<Value> exception = errors.ToException(env, exception_string);
397+
Local<Value> exception;
398+
if (!errors.ToException(env, exception_string).ToLocal(&exception))
399+
return;
395400
Local<Object> obj;
396401
if (!exception->ToObject(env->context()).ToLocal(&obj))
397402
return;
398-
error::Decorate(env, obj, err);
403+
if (error::Decorate(env, obj, err).IsNothing())
404+
return;
399405
env->isolate()->ThrowException(exception);
400406
}
401407

@@ -5872,7 +5878,7 @@ struct RandomBytesJob : public CryptoJob {
58725878

58735879
inline Local<Value> ToResult() const {
58745880
if (errors.empty()) return Undefined(env->isolate());
5875-
return errors.ToException(env);
5881+
return errors.ToException(env).ToLocalChecked();
58765882
}
58775883
};
58785884

@@ -6009,7 +6015,7 @@ struct ScryptJob : public CryptoJob {
60096015

60106016
inline Local<Value> ToResult() const {
60116017
if (errors.empty()) return Undefined(env->isolate());
6012-
return errors.ToException(env);
6018+
return errors.ToException(env).ToLocalChecked();
60136019
}
60146020

60156021
inline void Cleanse() {
@@ -6285,7 +6291,7 @@ class GenerateKeyPairJob : public CryptoJob {
62856291
if (errors_.empty())
62866292
errors_.Capture();
62876293
CHECK(!errors_.empty());
6288-
*err = errors_.ToException(env);
6294+
*err = errors_.ToException(env).ToLocalChecked();
62896295
*pubkey = Undefined(env->isolate());
62906296
*privkey = Undefined(env->isolate());
62916297
}

test/parallel/test-crypto-sign-verify.js

+47-1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,45 @@ const modSize = 1024;
2929
'instance when called without `new`');
3030
}
3131

32+
// Test handling of exceptional conditions
33+
{
34+
const library = {
35+
configurable: true,
36+
set() {
37+
throw new Error('bye, bye, library');
38+
}
39+
};
40+
Object.defineProperty(Object.prototype, 'library', library);
41+
42+
assert.throws(() => {
43+
crypto.createSign('sha1').sign(
44+
`-----BEGIN RSA PRIVATE KEY-----
45+
AAAAAAAAAAAA
46+
-----END RSA PRIVATE KEY-----`);
47+
}, { message: 'bye, bye, library' });
48+
49+
delete Object.prototype.library;
50+
51+
const errorStack = {
52+
configurable: true,
53+
set() {
54+
throw new Error('bye, bye, error stack');
55+
}
56+
};
57+
Object.defineProperty(Object.prototype, 'opensslErrorStack', errorStack);
58+
59+
assert.throws(() => {
60+
crypto.createSign('SHA1')
61+
.update('Test123')
62+
.sign({
63+
key: keyPem,
64+
padding: crypto.constants.RSA_PKCS1_OAEP_PADDING
65+
});
66+
}, { message: 'bye, bye, error stack' });
67+
68+
delete Object.prototype.opensslErrorStack;
69+
}
70+
3271
common.expectsError(
3372
() => crypto.createVerify('SHA256').verify({
3473
key: certPem,
@@ -300,7 +339,14 @@ common.expectsError(
300339
key: keyPem,
301340
padding: crypto.constants.RSA_PKCS1_OAEP_PADDING
302341
});
303-
}, /^Error:.*illegal or unsupported padding mode$/);
342+
}, {
343+
code: 'ERR_OSSL_RSA_ILLEGAL_OR_UNSUPPORTED_PADDING_MODE',
344+
message: /illegal or unsupported padding mode/,
345+
opensslErrorStack: [
346+
'error:06089093:digital envelope routines:EVP_PKEY_CTX_ctrl:' +
347+
'command not supported',
348+
],
349+
});
304350
}
305351

306352
// Test throws exception when key options is null

0 commit comments

Comments
 (0)