Skip to content

Commit b7b0768

Browse files
joyeecheungaduh95
authored andcommitted
tls: fix error stack conversion in cryptoErrorListToException()
The ncrypto move introduced regressions in cryptoErrorListToException() by passing in the size of the vector unnecessarily into the vector constructor and then use push_back() (which would result in a crash on dereferencing empty handles during later iteration) and having incorrect logic for checking the presence of an exception. This patch fixes it. PR-URL: #56554 Fixes: #56375 Refs: #53803 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
1 parent 4aa1afd commit b7b0768

File tree

2 files changed

+23
-4
lines changed

2 files changed

+23
-4
lines changed

src/crypto/crypto_util.cc

+5-4
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,8 @@ MaybeLocal<Value> cryptoErrorListToException(
237237
if (errors.size() > 1) {
238238
CHECK(exception->IsObject());
239239
Local<Object> exception_obj = exception.As<Object>();
240-
LocalVector<Value> stack(env->isolate(), errors.size() - 1);
240+
LocalVector<Value> stack(env->isolate());
241+
stack.reserve(errors.size() - 1);
241242

242243
// Iterate over all but the last error in the list.
243244
auto current = errors.begin();
@@ -255,9 +256,9 @@ MaybeLocal<Value> cryptoErrorListToException(
255256
Local<v8::Array> stackArray =
256257
v8::Array::New(env->isolate(), stack.data(), stack.size());
257258

258-
if (!exception_obj
259-
->Set(env->context(), env->openssl_error_stack(), stackArray)
260-
.IsNothing()) {
259+
if (exception_obj
260+
->Set(env->context(), env->openssl_error_stack(), stackArray)
261+
.IsNothing()) {
261262
return {};
262263
}
263264
}

test/parallel/test-tls-error-stack.js

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
'use strict';
2+
3+
// This tests that the crypto error stack can be correctly converted.
4+
const common = require('../common');
5+
if (!common.hasCrypto)
6+
common.skip('missing crypto');
7+
8+
const assert = require('assert');
9+
const tls = require('tls');
10+
11+
assert.throws(() => {
12+
tls.createSecureContext({ clientCertEngine: 'x' });
13+
}, (err) => {
14+
return err.name === 'Error' &&
15+
/could not load the shared library/.test(err.message) &&
16+
Array.isArray(err.opensslErrorStack) &&
17+
err.opensslErrorStack.length > 0;
18+
});

0 commit comments

Comments
 (0)