From 6327d67be3a939b1ce6ac8541504b1f6fc338fd9 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Tue, 30 Jul 2013 14:27:13 +0200 Subject: [PATCH] crypto: fix assert() on malformed hex input Use the StringBytes::IsValidString() function introduced in commit dce26cc to ensure that the input string meets the expectations of the other StringBytes functions before processing it further. Fixes the following assertion: Assertion failed: (str->Length() % 2 == 0 && "invalid hex string length"), function StorageSize, file ../../src/string_bytes.cc, line 301. Fixes #5725. --- src/node_crypto.cc | 42 +++++++++++++++++++++++++++----------- test/simple/test-crypto.js | 22 ++++++++++++++++++++ 2 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index a609c4f9f3..02b0660ef0 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -2265,10 +2265,13 @@ class Cipher : public ObjectWrap { unsigned char* out = 0; int out_len = 0, r; if (args[0]->IsString()) { + Local string = args[0].As(); enum encoding encoding = ParseEncoding(args[1], BINARY); - size_t buflen = StringBytes::StorageSize(args[0], encoding); + if (!StringBytes::IsValidString(string, encoding)) + return ThrowTypeError("Bad input string"); + size_t buflen = StringBytes::StorageSize(string, encoding); char* buf = new char[buflen]; - size_t written = StringBytes::Write(buf, buflen, args[0], encoding); + size_t written = StringBytes::Write(buf, buflen, string, encoding); r = cipher->CipherUpdate(buf, written, &out, &out_len); delete[] buf; } else { @@ -2574,10 +2577,13 @@ class Decipher : public ObjectWrap { unsigned char* out = 0; int out_len = 0, r; if (args[0]->IsString()) { + Local string = args[0].As(); enum encoding encoding = ParseEncoding(args[1], BINARY); - size_t buflen = StringBytes::StorageSize(args[0], encoding); + if (!StringBytes::IsValidString(string, encoding)) + return ThrowTypeError("Bad input string"); + size_t buflen = StringBytes::StorageSize(string, encoding); char* buf = new char[buflen]; - size_t written = StringBytes::Write(buf, buflen, args[0], encoding); + size_t written = StringBytes::Write(buf, buflen, string, encoding); r = cipher->DecipherUpdate(buf, written, &out, &out_len); delete[] buf; } else { @@ -2763,10 +2769,13 @@ class Hmac : public ObjectWrap { // Only copy the data if we have to, because it's a string int r; if (args[0]->IsString()) { + Local string = args[0].As(); enum encoding encoding = ParseEncoding(args[1], BINARY); - size_t buflen = StringBytes::StorageSize(args[0], encoding); + if (!StringBytes::IsValidString(string, encoding)) + return ThrowTypeError("Bad input string"); + size_t buflen = StringBytes::StorageSize(string, encoding); char* buf = new char[buflen]; - size_t written = StringBytes::Write(buf, buflen, args[0], encoding); + size_t written = StringBytes::Write(buf, buflen, string, encoding); r = hmac->HmacUpdate(buf, written); delete[] buf; } else { @@ -2892,10 +2901,13 @@ class Hash : public ObjectWrap { // Only copy the data if we have to, because it's a string int r; if (args[0]->IsString()) { + Local string = args[0].As(); enum encoding encoding = ParseEncoding(args[1], BINARY); - size_t buflen = StringBytes::StorageSize(args[0], encoding); + if (!StringBytes::IsValidString(string, encoding)) + return ThrowTypeError("Bad input string"); + size_t buflen = StringBytes::StorageSize(string, encoding); char* buf = new char[buflen]; - size_t written = StringBytes::Write(buf, buflen, args[0], encoding); + size_t written = StringBytes::Write(buf, buflen, string, encoding); r = hash->HashUpdate(buf, written); delete[] buf; } else { @@ -3055,10 +3067,13 @@ class Sign : public ObjectWrap { // Only copy the data if we have to, because it's a string int r; if (args[0]->IsString()) { + Local string = args[0].As(); enum encoding encoding = ParseEncoding(args[1], BINARY); - size_t buflen = StringBytes::StorageSize(args[0], encoding); + if (!StringBytes::IsValidString(string, encoding)) + return ThrowTypeError("Bad input string"); + size_t buflen = StringBytes::StorageSize(string, encoding); char* buf = new char[buflen]; - size_t written = StringBytes::Write(buf, buflen, args[0], encoding); + size_t written = StringBytes::Write(buf, buflen, string, encoding); r = sign->SignUpdate(buf, written); delete[] buf; } else { @@ -3280,10 +3295,13 @@ class Verify : public ObjectWrap { // Only copy the data if we have to, because it's a string int r; if (args[0]->IsString()) { + Local string = args[0].As(); enum encoding encoding = ParseEncoding(args[1], BINARY); - size_t buflen = StringBytes::StorageSize(args[0], encoding); + if (!StringBytes::IsValidString(string, encoding)) + return ThrowTypeError("Bad input string"); + size_t buflen = StringBytes::StorageSize(string, encoding); char* buf = new char[buflen]; - size_t written = StringBytes::Write(buf, buflen, args[0], encoding); + size_t written = StringBytes::Write(buf, buflen, string, encoding); r = verify->VerifyUpdate(buf, written); delete[] buf; } else { diff --git a/test/simple/test-crypto.js b/test/simple/test-crypto.js index 36f232d678..abb767f762 100644 --- a/test/simple/test-crypto.js +++ b/test/simple/test-crypto.js @@ -915,3 +915,25 @@ assert.throws(function() { c.update('update', 'utf-8'); c.final('utf8'); // Should not throw. })(); + +// Regression tests for #5725: hex input that's not a power of two should +// throw, not assert in C++ land. +assert.throws(function() { + crypto.createCipher('aes192', 'test').update('0', 'hex'); +}, /Bad input string/); + +assert.throws(function() { + crypto.createDecipher('aes192', 'test').update('0', 'hex'); +}, /Bad input string/); + +assert.throws(function() { + crypto.createHash('sha1').update('0', 'hex'); +}, /Bad input string/); + +assert.throws(function() { + crypto.createSign('RSA-SHA1').update('0', 'hex'); +}, /Bad input string/); + +assert.throws(function() { + crypto.createVerify('RSA-SHA1').update('0', 'hex'); +}, /Bad input string/);