buffer: throw if string is not a valid HEX string

As it is, if an invalid HEX string is passed to `Buffer` constructor,
it will only use the valid HEX values and ignore the rest. But, it also
throws an error when the length of the string is odd in length. This
patch throws an error if the string is not a valid HEX string.

Fixes: #3770

src/node_buffer.cc

@@ -622,9 +622,6 @@ void StringWrite(const FunctionCallbackInfo<Value>& args) {
 
   Local<String> str = args[0]->ToString(env->isolate());
 
-  if (encoding == HEX && str->Length() % 2 != 0)
-    return env->ThrowTypeError("Invalid hex string");
-
   size_t offset;
   size_t max_length;
 
@@ -639,6 +636,11 @@ void StringWrite(const FunctionCallbackInfo<Value>& args) {
   if (offset >= ts_obj_length)
     return env->ThrowRangeError("Offset is out of bounds");
 
+  if (StringBytes::IsValidString(env->isolate(), str, encoding) == false) {
+    if (encoding == HEX)
+      return env->ThrowTypeError("Invalid hex string");
+  }
+
   uint32_t written = StringBytes::Write(env->isolate(),
                                         ts_obj_data + offset,
                                         max_length,

src/string_bytes.cc

@@ -451,11 +451,36 @@ size_t StringBytes::Write(Isolate* isolate,
   return nbytes;
 }
 
+template <typename Type>
+bool IsValidHexString(Isolate* isolate, const Type* str, const size_t len) {
+  for (size_t i = 0; i < len; i += 1) {
+    if (!((str[i] >= '0' && str[i] <= '9') ||
+          (str[i] >= 'a' && str[i] <= 'f') ||
+          (str[i] >= 'A' && str[i] <= 'F')))
+      return false;
+  }
+  return true;
+}
+
+bool IsValidHexString(Isolate* isolate, Local<String> string) {
+  if (string->Length() % 2 != 0)
+    return false;
+
+  const char* str = nullptr;
+  size_t n = 0;
+  bool external = StringBytes::GetExternalParts(isolate, string, &str, &n);
+
+  if (external)
+    return IsValidHexString(isolate, str, n);
+
+  String::Value value(string);
+  return IsValidHexString(isolate, *value, value.length());
+}
 
 bool StringBytes::IsValidString(Isolate* isolate,
                                 Local<String> string,
                                 enum encoding enc) {
-  if (enc == HEX && string->Length() % 2 != 0)
+  if (enc == HEX && IsValidHexString(isolate, string) == false)
     return false;
   // TODO(bnoordhuis) Add BASE64 check?
   return true;

test/parallel/test-buffer.js

@@ -741,6 +741,15 @@ for (let i = 0; i < 256; i++) {
   assert.equal(b2, b4);
 }
 
+// invalid HEX strings should throw an error
+assert.throws(() => new Buffer('xx', 'hex'), /TypeError: Invalid hex string/);
+// following will fail even though it has zero as the first character, because
+// `x` is an invalid hexa-decimal value
+assert.throws(() => new Buffer('0x', 'hex'), /TypeError: Invalid hex string/);
+// following invalid HEX strings will fail because of the odd length
+assert.throws(() => new Buffer('0', 'hex'), /TypeError: Invalid hex string/);
+assert.throws(() => new Buffer('000', 'hex'), /TypeError: Invalid hex string/);
+
 function buildBuffer(data) {
   if (Array.isArray(data)) {
     var buffer = new Buffer(data.length);