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.

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 57fbba7..bea9408 100644 (file)
--- 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<Value> CipherUpdate(const Arguments& args) {
     Cipher *cipher = ObjectWrap::Unwrap<Cipher>(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<Decipher>(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<Hash>(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<Verify>(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<Verify>(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 6083cac..abf77ee 100644 (file)
--- 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/);