Skip to content

Commit 977b46b

Browse files
rfkMylesBorins
authored andcommitted
crypto: clear err stack after ECDH::BufferToPoint
Functions that call `ECDH::BufferToPoint` were not clearing the error stack on failure, so an invalid key could leave leftover error state and cause subsequent (unrelated) signing operations to fail. PR-URL: #13275 Backport-PR-URL: #13399 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 161dce3 commit 977b46b

File tree

2 files changed

+24
-0
lines changed

2 files changed

+24
-0
lines changed

src/node_crypto.cc

+4
Original file line numberDiff line numberDiff line change
@@ -4916,6 +4916,8 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
49164916
ECDH* ecdh;
49174917
ASSIGN_OR_RETURN_UNWRAP(&ecdh, args.Holder());
49184918

4919+
MarkPopErrorOnReturn mark_pop_error_on_return;
4920+
49194921
EC_POINT* pub = ecdh->BufferToPoint(Buffer::Data(args[0]),
49204922
Buffer::Length(args[0]));
49214923
if (pub == nullptr)
@@ -5038,6 +5040,8 @@ void ECDH::SetPublicKey(const FunctionCallbackInfo<Value>& args) {
50385040

50395041
THROW_AND_RETURN_IF_NOT_BUFFER(args[0]);
50405042

5043+
MarkPopErrorOnReturn mark_pop_error_on_return;
5044+
50415045
EC_POINT* pub = ecdh->BufferToPoint(Buffer::Data(args[0].As<Object>()),
50425046
Buffer::Length(args[0].As<Object>()));
50435047
if (pub == nullptr)

test/parallel/test-crypto-dh.js

+20
Original file line numberDiff line numberDiff line change
@@ -188,3 +188,23 @@ ecdh4.setPublicKey(ecdh1.getPublicKey());
188188
assert.throws(function() {
189189
ecdh4.setPublicKey(ecdh3.getPublicKey());
190190
});
191+
192+
// Use of invalid keys was not cleaning up ERR stack, and was causing
193+
// unexpected failure in subsequent signing operations.
194+
var ecdh5 = crypto.createECDH('prime256v1');
195+
var invalidKey = Buffer.alloc(65);
196+
invalidKey.fill('\0');
197+
ecdh5.generateKeys();
198+
assert.throws(() => {
199+
ecdh5.computeSecret(invalidKey);
200+
}, /^Error: Failed to translate Buffer to a EC_POINT$/);
201+
// Check that signing operations are not impacted by the above error.
202+
const ecPrivateKey =
203+
'-----BEGIN EC PRIVATE KEY-----\n' +
204+
'MHcCAQEEIF+jnWY1D5kbVYDNvxxo/Y+ku2uJPDwS0r/VuPZQrjjVoAoGCCqGSM49\n' +
205+
'AwEHoUQDQgAEurOxfSxmqIRYzJVagdZfMMSjRNNhB8i3mXyIMq704m2m52FdfKZ2\n' +
206+
'pQhByd5eyj3lgZ7m7jbchtdgyOF8Io/1ng==\n' +
207+
'-----END EC PRIVATE KEY-----';
208+
assert.doesNotThrow(() => {
209+
crypto.createSign('SHA256').sign(ecPrivateKey);
210+
});

0 commit comments

Comments
 (0)