From 9d4c5a12f499fd59156e16efce17f019e16708d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Geisendo=CC=88rfer?= Date: Mon, 14 Mar 2011 11:16:35 +0100 Subject: [PATCH] Crypto update should only accept strings / buffers I have seen a lot of people trying to pass objects to crypto's update functions, assuming that it would somehow serialize the object before hashing. In reality, the object was converted to '[object Object]' which was then hashed, without any error message showing. This patch modifies the DecodeBytes function (used exclusively by crypto at this point) to complain when receiving anything but a string or buffer. Overall this should be a less-suprising, more robust behavior. --- src/node_crypto.cc | 26 ++++++++++++++++++++++++-- test/simple/test-crypto.js | 4 ++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 57fbba71fc..bea94087e1 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -16,6 +16,11 @@ # define OPENSSL_CONST #endif +#define ASSERT_IS_STRING_OR_BUFFER(val) \ + if (!val->IsString() && !Buffer::HasInstance(val)) { \ + return ThrowException(Exception::TypeError(String::New("Not a string or buffer"))); \ + } + namespace node { namespace crypto { @@ -1420,6 +1425,7 @@ class Cipher : public ObjectWrap { "Must give cipher-type, key"))); } + ASSERT_IS_STRING_OR_BUFFER(args[1]); ssize_t key_buf_len = DecodeBytes(args[1], BINARY); if (key_buf_len < 0) { @@ -1456,6 +1462,8 @@ class Cipher : public ObjectWrap { return ThrowException(Exception::Error(String::New( "Must give cipher-type, key, and iv as argument"))); } + + ASSERT_IS_STRING_OR_BUFFER(args[1]); ssize_t key_len = DecodeBytes(args[1], BINARY); if (key_len < 0) { @@ -1463,6 +1471,7 @@ class Cipher : public ObjectWrap { return ThrowException(exception); } + ASSERT_IS_STRING_OR_BUFFER(args[2]); ssize_t iv_len = DecodeBytes(args[2], BINARY); if (iv_len < 0) { @@ -1492,12 +1501,13 @@ class Cipher : public ObjectWrap { return args.This(); } - static Handle CipherUpdate(const Arguments& args) { Cipher *cipher = ObjectWrap::Unwrap(args.This()); HandleScope scope; + ASSERT_IS_STRING_OR_BUFFER(args[0]); + enum encoding enc = ParseEncoding(args[1]); ssize_t len = DecodeBytes(args[0], enc); @@ -1781,6 +1791,7 @@ class Decipher : public ObjectWrap { "Must give cipher-type, key as argument"))); } + ASSERT_IS_STRING_OR_BUFFER(args[1]); ssize_t key_len = DecodeBytes(args[1], BINARY); if (key_len < 0) { @@ -1818,6 +1829,7 @@ class Decipher : public ObjectWrap { "Must give cipher-type, key, and iv as argument"))); } + ASSERT_IS_STRING_OR_BUFFER(args[1]); ssize_t key_len = DecodeBytes(args[1], BINARY); if (key_len < 0) { @@ -1825,6 +1837,7 @@ class Decipher : public ObjectWrap { return ThrowException(exception); } + ASSERT_IS_STRING_OR_BUFFER(args[2]); ssize_t iv_len = DecodeBytes(args[2], BINARY); if (iv_len < 0) { @@ -1859,6 +1872,8 @@ class Decipher : public ObjectWrap { Decipher *cipher = ObjectWrap::Unwrap(args.This()); + ASSERT_IS_STRING_OR_BUFFER(args[0]); + ssize_t len = DecodeBytes(args[0], BINARY); if (len < 0) { return ThrowException(Exception::Error(String::New( @@ -2160,6 +2175,7 @@ class Hmac : public ObjectWrap { "Must give hashtype string as argument"))); } + ASSERT_IS_STRING_OR_BUFFER(args[1]); ssize_t len = DecodeBytes(args[1], BINARY); if (len < 0) { @@ -2189,6 +2205,7 @@ class Hmac : public ObjectWrap { HandleScope scope; + ASSERT_IS_STRING_OR_BUFFER(args[0]); enum encoding enc = ParseEncoding(args[1]); ssize_t len = DecodeBytes(args[0], enc); @@ -2336,6 +2353,7 @@ class Hash : public ObjectWrap { Hash *hash = ObjectWrap::Unwrap(args.This()); + ASSERT_IS_STRING_OR_BUFFER(args[0]); enum encoding enc = ParseEncoding(args[1]); ssize_t len = DecodeBytes(args[0], enc); @@ -2527,6 +2545,7 @@ class Sign : public ObjectWrap { HandleScope scope; + ASSERT_IS_STRING_OR_BUFFER(args[0]); enum encoding enc = ParseEncoding(args[1]); ssize_t len = DecodeBytes(args[0], enc); @@ -2573,6 +2592,7 @@ class Sign : public ObjectWrap { md_len = 8192; // Maximum key size is 8192 bits md_value = new unsigned char[md_len]; + ASSERT_IS_STRING_OR_BUFFER(args[0]); ssize_t len = DecodeBytes(args[0], BINARY); if (len < 0) { @@ -2740,6 +2760,7 @@ class Verify : public ObjectWrap { Verify *verify = ObjectWrap::Unwrap(args.This()); + ASSERT_IS_STRING_OR_BUFFER(args[0]); enum encoding enc = ParseEncoding(args[1]); ssize_t len = DecodeBytes(args[0], enc); @@ -2778,6 +2799,7 @@ class Verify : public ObjectWrap { Verify *verify = ObjectWrap::Unwrap(args.This()); + ASSERT_IS_STRING_OR_BUFFER(args[0]); ssize_t klen = DecodeBytes(args[0], BINARY); if (klen < 0) { @@ -2789,7 +2811,7 @@ class Verify : public ObjectWrap { ssize_t kwritten = DecodeWrite(kbuf, klen, args[0], BINARY); assert(kwritten == klen); - + ASSERT_IS_STRING_OR_BUFFER(args[1]); ssize_t hlen = DecodeBytes(args[1], BINARY); if (hlen < 0) { diff --git a/test/simple/test-crypto.js b/test/simple/test-crypto.js index 6083cac64f..abf77eec38 100644 --- a/test/simple/test-crypto.js +++ b/test/simple/test-crypto.js @@ -119,3 +119,7 @@ txt += decipher.final('utf8'); assert.equal(txt, plaintext, 'encryption and decryption with key and iv'); +// update() should only take buffers / strings +assert.throws(function() { + crypto.createHash('sha1').update({foo: 'bar'}); +}, /string or buffer/);