From 3640ba4acb201323fc69c555cb04f03b50d43cee Mon Sep 17 00:00:00 2001
From: Ryan Kelly <ryan@rfk.id.au>
Date: Mon, 29 May 2017 14:20:04 +1000
Subject: [PATCH] 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: https://github.com/nodejs/node/pull/13275
Backport-PR-URL: https://github.com/nodejs/node/pull/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>
---
 src/node_crypto.cc              |  4 ++++
 test/parallel/test-crypto-dh.js | 20 ++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index d447e25b637d91..13af291dfc0861 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -4916,6 +4916,8 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
   ECDH* ecdh;
   ASSIGN_OR_RETURN_UNWRAP(&ecdh, args.Holder());
 
+  MarkPopErrorOnReturn mark_pop_error_on_return;
+
   EC_POINT* pub = ecdh->BufferToPoint(Buffer::Data(args[0]),
                                       Buffer::Length(args[0]));
   if (pub == nullptr)
@@ -5038,6 +5040,8 @@ void ECDH::SetPublicKey(const FunctionCallbackInfo<Value>& args) {
 
   THROW_AND_RETURN_IF_NOT_BUFFER(args[0]);
 
+  MarkPopErrorOnReturn mark_pop_error_on_return;
+
   EC_POINT* pub = ecdh->BufferToPoint(Buffer::Data(args[0].As<Object>()),
                                       Buffer::Length(args[0].As<Object>()));
   if (pub == nullptr)
diff --git a/test/parallel/test-crypto-dh.js b/test/parallel/test-crypto-dh.js
index e6b1de2475c897..29d33741ee0ce8 100644
--- a/test/parallel/test-crypto-dh.js
+++ b/test/parallel/test-crypto-dh.js
@@ -188,3 +188,23 @@ ecdh4.setPublicKey(ecdh1.getPublicKey());
 assert.throws(function() {
   ecdh4.setPublicKey(ecdh3.getPublicKey());
 });
+
+// Use of invalid keys was not cleaning up ERR stack, and was causing
+// unexpected failure in subsequent signing operations.
+var ecdh5 = crypto.createECDH('prime256v1');
+var invalidKey = Buffer.alloc(65);
+invalidKey.fill('\0');
+ecdh5.generateKeys();
+assert.throws(() => {
+  ecdh5.computeSecret(invalidKey);
+}, /^Error: Failed to translate Buffer to a EC_POINT$/);
+// Check that signing operations are not impacted by the above error.
+const ecPrivateKey =
+  '-----BEGIN EC PRIVATE KEY-----\n' +
+  'MHcCAQEEIF+jnWY1D5kbVYDNvxxo/Y+ku2uJPDwS0r/VuPZQrjjVoAoGCCqGSM49\n' +
+  'AwEHoUQDQgAEurOxfSxmqIRYzJVagdZfMMSjRNNhB8i3mXyIMq704m2m52FdfKZ2\n' +
+  'pQhByd5eyj3lgZ7m7jbchtdgyOF8Io/1ng==\n' +
+  '-----END EC PRIVATE KEY-----';
+assert.doesNotThrow(() => {
+  crypto.createSign('SHA256').sign(ecPrivateKey);
+});