Skip to content

Commit 04fb72f

Browse files
rfkgibfahn
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: nodejs#13275 Backport-PR-URL: nodejs#13397 Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 321c90f commit 04fb72f

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
@@ -5011,6 +5011,8 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
50115011
ECDH* ecdh;
50125012
ASSIGN_OR_RETURN_UNWRAP(&ecdh, args.Holder());
50135013

5014+
MarkPopErrorOnReturn mark_pop_error_on_return;
5015+
50145016
if (!ecdh->IsKeyPairValid())
50155017
return env->ThrowError("Invalid key pair");
50165018

@@ -5160,6 +5162,8 @@ void ECDH::SetPublicKey(const FunctionCallbackInfo<Value>& args) {
51605162

51615163
THROW_AND_RETURN_IF_NOT_BUFFER(args[0], "Public key");
51625164

5165+
MarkPopErrorOnReturn mark_pop_error_on_return;
5166+
51635167
EC_POINT* pub = ecdh->BufferToPoint(Buffer::Data(args[0].As<Object>()),
51645168
Buffer::Length(args[0].As<Object>()));
51655169
if (pub == nullptr)

test/parallel/test-crypto-dh.js

+20
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,26 @@ ecdh5.setPrivateKey(cafebabeKey, 'hex');
270270
assert.strictEqual(ecdh5.getPrivateKey('hex'), cafebabeKey);
271271
});
272272

273+
// Use of invalid keys was not cleaning up ERR stack, and was causing
274+
// unexpected failure in subsequent signing operations.
275+
const ecdh6 = crypto.createECDH('prime256v1');
276+
const invalidKey = Buffer.alloc(65);
277+
invalidKey.fill('\0');
278+
ecdh6.generateKeys();
279+
assert.throws(() => {
280+
ecdh6.computeSecret(invalidKey);
281+
}, /^Error: Failed to translate Buffer to a EC_POINT$/);
282+
// Check that signing operations are not impacted by the above error.
283+
const ecPrivateKey =
284+
'-----BEGIN EC PRIVATE KEY-----\n' +
285+
'MHcCAQEEIF+jnWY1D5kbVYDNvxxo/Y+ku2uJPDwS0r/VuPZQrjjVoAoGCCqGSM49\n' +
286+
'AwEHoUQDQgAEurOxfSxmqIRYzJVagdZfMMSjRNNhB8i3mXyIMq704m2m52FdfKZ2\n' +
287+
'pQhByd5eyj3lgZ7m7jbchtdgyOF8Io/1ng==\n' +
288+
'-----END EC PRIVATE KEY-----';
289+
assert.doesNotThrow(() => {
290+
crypto.createSign('SHA256').sign(ecPrivateKey);
291+
});
292+
273293
// invalid test: curve argument is undefined
274294
assert.throws(() => {
275295
crypto.createECDH();

0 commit comments

Comments
 (0)