Skip to content

Commit

Permalink
crypto: handle exceptions in hmac/hash.digest
Browse files Browse the repository at this point in the history
Forced conversion of the encoding parameter to a string within
crypto.js, fixing segmentation faults in node_crypto.cc.

Fixes: #9819
PR-URL: #12164
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
tniessen authored and MylesBorins committed May 15, 2017
1 parent f1ea367 commit 7678da4
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 14 deletions.
3 changes: 2 additions & 1 deletion lib/crypto.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ Hash.prototype.update = function(data, encoding) {

Hash.prototype.digest = function(outputEncoding) {
outputEncoding = outputEncoding || exports.DEFAULT_ENCODING;
return this._handle.digest(outputEncoding);
// Explicit conversion for backward compatibility.
return this._handle.digest(`${outputEncoding}`);
};


Expand Down
2 changes: 2 additions & 0 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1483,6 +1483,8 @@ enum encoding ParseEncoding(const char* encoding,
enum encoding ParseEncoding(Isolate* isolate,
Local<Value> encoding_v,
enum encoding default_encoding) {
CHECK(!encoding_v.IsEmpty());

if (!encoding_v->IsString())
return default_encoding;

Expand Down
20 changes: 7 additions & 13 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3783,9 +3783,8 @@ void Hmac::HmacDigest(const FunctionCallbackInfo<Value>& args) {

enum encoding encoding = BUFFER;
if (args.Length() >= 1) {
encoding = ParseEncoding(env->isolate(),
args[0]->ToString(env->isolate()),
BUFFER);
CHECK(args[0]->IsString());
encoding = ParseEncoding(env->isolate(), args[0], BUFFER);
}

unsigned char* md_value = nullptr;
Expand Down Expand Up @@ -3907,9 +3906,8 @@ void Hash::HashDigest(const FunctionCallbackInfo<Value>& args) {

enum encoding encoding = BUFFER;
if (args.Length() >= 1) {
encoding = ParseEncoding(env->isolate(),
args[0]->ToString(env->isolate()),
BUFFER);
CHECK(args[0]->IsString());
encoding = ParseEncoding(env->isolate(), args[0], BUFFER);
}

unsigned char md_value[EVP_MAX_MD_SIZE];
Expand Down Expand Up @@ -4132,10 +4130,8 @@ void Sign::SignFinal(const FunctionCallbackInfo<Value>& 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]);
Expand Down Expand Up @@ -4348,9 +4344,7 @@ void Verify::VerifyFinal(const FunctionCallbackInfo<Value>& 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);
Expand Down
24 changes: 24 additions & 0 deletions test/parallel/test-regress-GH-9819.js
Original file line number Diff line number Diff line change
@@ -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');
}));
});

0 comments on commit 7678da4

Please sign in to comment.