Skip to content

Commit

Permalink
tls: fix error stack conversion in cryptoErrorListToException()
Browse files Browse the repository at this point in the history
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: nodejs#56554
Fixes: nodejs#56375
Refs: nodejs#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>
  • Loading branch information
joyeecheung authored and Ceres6 committed Jan 13, 2025
1 parent ec65d56 commit afdd593
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 4 deletions.
9 changes: 5 additions & 4 deletions src/crypto/crypto_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,8 @@ MaybeLocal<Value> cryptoErrorListToException(
if (errors.size() > 1) {
CHECK(exception->IsObject());
Local<Object> exception_obj = exception.As<Object>();
LocalVector<Value> stack(env->isolate(), errors.size() - 1);
LocalVector<Value> stack(env->isolate());
stack.reserve(errors.size() - 1);

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

if (!exception_obj
->Set(env->context(), env->openssl_error_stack(), stackArray)
.IsNothing()) {
if (exception_obj
->Set(env->context(), env->openssl_error_stack(), stackArray)
.IsNothing()) {
return {};
}
}
Expand Down
18 changes: 18 additions & 0 deletions test/parallel/test-tls-error-stack.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
'use strict';

// This tests that the crypto error stack can be correctly converted.
const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');

const assert = require('assert');
const tls = require('tls');

assert.throws(() => {
tls.createSecureContext({ clientCertEngine: 'x' });
}, (err) => {
return err.name === 'Error' &&
/could not load the shared library/.test(err.message) &&
Array.isArray(err.opensslErrorStack) &&
err.opensslErrorStack.length > 0;
});

0 comments on commit afdd593

Please # to comment.