src: add Malloc() size param + overflow detection

Adds an optional second parameter to `node::Malloc()` and
an optional third parameter to `node::Realloc()` giving the
size/number of items to be allocated, in the style of `calloc(3)`.

Use a proper overflow check using division;
the previous `CHECK_GE(n * size, n);` would not detect all cases
of overflow (e.g. `size == SIZE_MAX / 2 && n == 3`).

Backport-PR-URL: https://github.com/nodejs/node/pull/16587
PR-URL: https://github.com/nodejs/node/pull/8482
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>

4 files changed, 25 insertions, 17 deletions

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 99ed0ddf08..fefd471c6b 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -5826,11 +5826,10 @@ void GetCurves(const FunctionCallbackInfo<Value>& args) {
   const size_t num_curves = EC_get_builtin_curves(nullptr, 0);
   Local<Array> arr = Array::New(env->isolate(), num_curves);
   EC_builtin_curve* curves;
-  size_t alloc_size;
 
   if (num_curves) {
-    alloc_size = sizeof(*curves) * num_curves;
-    curves = static_cast<EC_builtin_curve*>(node::Malloc(alloc_size));
+    curves = static_cast<EC_builtin_curve*>(node::Malloc(sizeof(*curves),
+                                                         num_curves));
 
     CHECK_NE(curves, nullptr);
 
diff --git a/src/string_bytes.cc b/src/string_bytes.cc
index 9d1619d864..771ef03400 100644
--- a/src/string_bytes.cc
+++ b/src/string_bytes.cc
@@ -54,7 +54,7 @@ class ExternString: public ResourceType {
       return scope.Escape(String::Empty(isolate));
 
     TypeName* new_data =
-        static_cast<TypeName*>(node::Malloc(length * sizeof(*new_data)));
+        static_cast<TypeName*>(node::Malloc(length, sizeof(*new_data)));
     if (new_data == nullptr) {
       return Local<String>();
     }
diff --git a/src/util-inl.h b/src/util-inl.h
index 27bced48fe..8f23a59651 100644
--- a/src/util-inl.h
+++ b/src/util-inl.h
@@ -320,6 +320,14 @@ bool StringEqualNoCaseN(const char* a, const char* b, size_t length) {
   return true;
 }
 
+inline size_t MultiplyWithOverflowCheck(size_t a, size_t b) {
+  size_t ret = a * b;
+  if (a != 0)
+    CHECK_EQ(b, ret / a);
+
+  return ret;
+}
+
 // These should be used in our code as opposed to the native
 // versions as they abstract out some platform and or
 // compiler version specific functionality.
@@ -327,24 +335,28 @@ bool StringEqualNoCaseN(const char* a, const char* b, size_t length) {
 // that the standard allows them to either return a unique pointer or a
 // nullptr for zero-sized allocation requests.  Normalize by always using
 // a nullptr.
-void* Realloc(void* pointer, size_t size) {
-  if (size == 0) {
+void* Realloc(void* pointer, size_t n, size_t size) {
+  size_t full_size = MultiplyWithOverflowCheck(size, n);
+
+  if (full_size == 0) {
     free(pointer);
     return nullptr;
   }
-  return realloc(pointer, size);
+
+  return realloc(pointer, full_size);
 }
 
 // As per spec realloc behaves like malloc if passed nullptr.
-void* Malloc(size_t size) {
+void* Malloc(size_t n, size_t size) {
+  if (n == 0) n = 1;
   if (size == 0) size = 1;
-  return Realloc(nullptr, size);
+  return Realloc(nullptr, n, size);
 }
 
 void* Calloc(size_t n, size_t size) {
   if (n == 0) n = 1;
   if (size == 0) size = 1;
-  CHECK_GE(n * size, n);  // Overflow guard.
+  MultiplyWithOverflowCheck(size, n);
   return calloc(n, size);
 }
 
diff --git a/src/util.h b/src/util.h
index f415141a58..38c17b390e 100644
--- a/src/util.h
+++ b/src/util.h
@@ -31,9 +31,9 @@ namespace node {
 // that the standard allows them to either return a unique pointer or a
 // nullptr for zero-sized allocation requests.  Normalize by always using
 // a nullptr.
-inline void* Realloc(void* pointer, size_t size);
-inline void* Malloc(size_t size);
-inline void* Calloc(size_t n, size_t size);
+inline void* Realloc(void* pointer, size_t n, size_t size = 1);
+inline void* Malloc(size_t n, size_t size = 1);
+inline void* Calloc(size_t n, size_t size = 1);
 
 #ifdef __GNUC__
 #define NO_RETURN __attribute__((noreturn))
@@ -294,10 +294,7 @@ class MaybeStackBuffer {
     if (storage <= kStackStorageSize) {
       buf_ = buf_st_;
     } else {
-      // Guard against overflow.
-      CHECK_LE(storage, sizeof(T) * storage);
-
-      buf_ = static_cast<T*>(Malloc(sizeof(T) * storage));
+      buf_ = static_cast<T*>(Malloc(sizeof(T), storage));
       CHECK_NE(buf_, nullptr);
     }