From 768c98627f01aa2ad055b21479102b8d2e6d5a9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Sun, 2 Apr 2017 15:00:32 +0200 Subject: [PATCH 1/4] crypto: handle exceptions in hmac/hash.digest Forced conversion of the encoding parameter to a string within crypto.js, fixing segmentation faults in node_crypto.cc. Fixes: https://github.com/nodejs/node/issues/9819 --- lib/crypto.js | 3 ++- src/node.cc | 2 ++ src/node_crypto.cc | 18 +++++------------- 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/lib/crypto.js b/lib/crypto.js index 3e7ed5e9c86960..ca3a5ce4711bc0 100644 --- a/lib/crypto.js +++ b/lib/crypto.js @@ -100,7 +100,8 @@ Hash.prototype.update = function update(data, encoding) { Hash.prototype.digest = function digest(outputEncoding) { outputEncoding = outputEncoding || exports.DEFAULT_ENCODING; - return this._handle.digest(outputEncoding); + // Explicit conversion for backward compatibility + return this._handle.digest(String(outputEncoding)); }; diff --git a/src/node.cc b/src/node.cc index 77e6a5826ee957..8b143c4b0f52b3 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1481,6 +1481,8 @@ enum encoding ParseEncoding(const char* encoding, enum encoding ParseEncoding(Isolate* isolate, Local encoding_v, enum encoding default_encoding) { + CHECK(!encoding_v.IsEmpty()); + if (!encoding_v->IsString()) return default_encoding; diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 45b06eaff50385..0c02e560cb0689 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -3797,9 +3797,7 @@ void Hmac::HmacDigest(const FunctionCallbackInfo& args) { enum encoding encoding = BUFFER; if (args.Length() >= 1) { - encoding = ParseEncoding(env->isolate(), - args[0]->ToString(env->isolate()), - BUFFER); + encoding = ParseEncoding(env->isolate(), args[0], BUFFER); } unsigned char* md_value = nullptr; @@ -3921,9 +3919,7 @@ void Hash::HashDigest(const FunctionCallbackInfo& args) { enum encoding encoding = BUFFER; if (args.Length() >= 1) { - encoding = ParseEncoding(env->isolate(), - args[0]->ToString(env->isolate()), - BUFFER); + encoding = ParseEncoding(env->isolate(), args[0], BUFFER); } unsigned char md_value[EVP_MAX_MD_SIZE]; @@ -4201,10 +4197,8 @@ void Sign::SignFinal(const FunctionCallbackInfo& args) { unsigned int len = args.Length(); enum encoding encoding = BUFFER; - if (len >= 2 && args[1]->IsString()) { - encoding = ParseEncoding(env->isolate(), - args[1]->ToString(env->isolate()), - BUFFER); + if (len >= 2) { + encoding = ParseEncoding(env->isolate(), args[1], BUFFER); } node::Utf8Value passphrase(env->isolate(), args[2]); @@ -4452,9 +4446,7 @@ void Verify::VerifyFinal(const FunctionCallbackInfo& args) { enum encoding encoding = UTF8; if (args.Length() >= 3) { - encoding = ParseEncoding(env->isolate(), - args[2]->ToString(env->isolate()), - UTF8); + encoding = ParseEncoding(env->isolate(), args[2], UTF8); } ssize_t hlen = StringBytes::Size(env->isolate(), args[1], encoding); From 23b71f54bef107d775a28038479b76569547e670 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Mon, 3 Apr 2017 13:37:10 +0200 Subject: [PATCH 2/4] Disallow symbols as encoding --- lib/crypto.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/crypto.js b/lib/crypto.js index ca3a5ce4711bc0..41810fe32e8444 100644 --- a/lib/crypto.js +++ b/lib/crypto.js @@ -101,7 +101,7 @@ Hash.prototype.update = function update(data, encoding) { Hash.prototype.digest = function digest(outputEncoding) { outputEncoding = outputEncoding || exports.DEFAULT_ENCODING; // Explicit conversion for backward compatibility - return this._handle.digest(String(outputEncoding)); + return this._handle.digest(`${outputEncoding}`); }; From 23f95ab393e519336cea4d514ee0bb49016fc7e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Mon, 3 Apr 2017 14:02:26 +0200 Subject: [PATCH 3/4] Add checks and regression test --- src/node_crypto.cc | 2 ++ test/parallel/test-regress-GH-9819.js | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 test/parallel/test-regress-GH-9819.js diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 0c02e560cb0689..0f9ed3434eca09 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -3797,6 +3797,7 @@ void Hmac::HmacDigest(const FunctionCallbackInfo& args) { enum encoding encoding = BUFFER; if (args.Length() >= 1) { + CHECK(args[0]->IsString()); encoding = ParseEncoding(env->isolate(), args[0], BUFFER); } @@ -3919,6 +3920,7 @@ void Hash::HashDigest(const FunctionCallbackInfo& args) { enum encoding encoding = BUFFER; if (args.Length() >= 1) { + CHECK(args[0]->IsString()); encoding = ParseEncoding(env->isolate(), args[0], BUFFER); } diff --git a/test/parallel/test-regress-GH-9819.js b/test/parallel/test-regress-GH-9819.js new file mode 100644 index 00000000000000..f043bc3b2848e7 --- /dev/null +++ b/test/parallel/test-regress-GH-9819.js @@ -0,0 +1,24 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const execFile = require('child_process').execFile; + +if (!common.hasCrypto) { + common.skip('missing crypto'); + return; +} + +const setup = 'const enc = { toString: () => { throw new Error("xyz"); } };'; + +const scripts = [ + 'crypto.createHash("sha256").digest(enc)', + 'crypto.createHmac("sha256", "msg").digest(enc)' +]; + +scripts.forEach((script) => { + const node = process.execPath; + const code = setup + ';' + script; + execFile(node, [ '-e', code ], common.mustCall((err, stdout, stderr) => { + assert(stderr.includes('Error: xyz'), 'digest crashes'); + })); +}); From 27d359cae4020cecc413e6d3008f74bd68c3dbc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Wed, 5 Apr 2017 10:54:47 +0200 Subject: [PATCH 4/4] Punctuate commit in crypto.js --- lib/crypto.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/crypto.js b/lib/crypto.js index 41810fe32e8444..0785ab617ea376 100644 --- a/lib/crypto.js +++ b/lib/crypto.js @@ -100,7 +100,7 @@ Hash.prototype.update = function update(data, encoding) { Hash.prototype.digest = function digest(outputEncoding) { outputEncoding = outputEncoding || exports.DEFAULT_ENCODING; - // Explicit conversion for backward compatibility + // Explicit conversion for backward compatibility. return this._handle.digest(`${outputEncoding}`); };