napi: fix memory corruption vulnerability

Fixes: https://hackerone.com/reports/784186 CVE-ID: CVE-2020-8174 PR-URL: nodejs-private/node-private#195 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
nodejs · Jun 2, 2020 · 7dd8982 · 7dd8982
1 parent c6d0bda
commit 7dd8982
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 3 deletions.
diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc
@@ -2209,7 +2209,7 @@ napi_status napi_get_value_string_latin1(napi_env env,
   if (!buf) {
     CHECK_ARG(env, result);
     *result = val.As<v8::String>()->Length();
-  } else {
+  } else if (bufsize != 0) {
     int copied =
         val.As<v8::String>()->WriteOneByte(env->isolate,
                                            reinterpret_cast<uint8_t*>(buf),
@@ -2221,6 +2221,8 @@ napi_status napi_get_value_string_latin1(napi_env env,
     if (result != nullptr) {
       *result = copied;
     }
+  } else if (result != nullptr) {
+    *result = 0;
   }
 
   return napi_clear_last_error(env);
@@ -2248,7 +2250,7 @@ napi_status napi_get_value_string_utf8(napi_env env,
   if (!buf) {
     CHECK_ARG(env, result);
     *result = val.As<v8::String>()->Utf8Length(env->isolate);
-  } else {
+  } else if (bufsize != 0) {
     int copied = val.As<v8::String>()->WriteUtf8(
         env->isolate,
         buf,
@@ -2260,6 +2262,8 @@ napi_status napi_get_value_string_utf8(napi_env env,
     if (result != nullptr) {
       *result = copied;
     }
+  } else if (result != nullptr) {
+    *result = 0;
   }
 
   return napi_clear_last_error(env);
@@ -2288,7 +2292,7 @@ napi_status napi_get_value_string_utf16(napi_env env,
     CHECK_ARG(env, result);
     // V8 assumes UTF-16 length is the same as the number of characters.
     *result = val.As<v8::String>()->Length();
-  } else {
+  } else if (bufsize != 0) {
     int copied = val.As<v8::String>()->Write(env->isolate,
                                              reinterpret_cast<uint16_t*>(buf),
                                              0,
@@ -2299,6 +2303,8 @@ napi_status napi_get_value_string_utf16(napi_env env,
     if (result != nullptr) {
       *result = copied;
     }
+  } else if (result != nullptr) {
+    *result = 0;
   }
 
   return napi_clear_last_error(env);

diff --git a/test/js-native-api/test_string/test.js b/test/js-native-api/test_string/test.js
@@ -81,3 +81,5 @@ assert.throws(() => {
 assert.throws(() => {
   test_string.TestLargeUtf16();
 }, /^Error: Invalid argument$/);
+
+test_string.TestMemoryCorruption(' '.repeat(64 * 1024));
diff --git a/test/js-native-api/test_string/test_string.c b/test/js-native-api/test_string/test_string.c
@@ -1,4 +1,5 @@
 #include <limits.h>  // INT_MAX
+#include <string.h>
 #include <js_native_api.h>
 #include "../common.h"
 
@@ -244,6 +245,24 @@ static napi_value TestLargeUtf16(napi_env env, napi_callback_info info) {
   return output;
 }
 
+static napi_value TestMemoryCorruption(napi_env env, napi_callback_info info) {
+  size_t argc = 1;
+  napi_value args[1];
+  NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
+
+  NAPI_ASSERT(env, argc == 1, "Wrong number of arguments");
+
+  char buf[10] = { 0 };
+  NAPI_CALL(env, napi_get_value_string_utf8(env, args[0], buf, 0, NULL));
+
+  char zero[10] = { 0 };
+  if (memcmp(buf, zero, sizeof(buf)) != 0) {
+    NAPI_CALL(env, napi_throw_error(env, NULL, "Buffer overwritten"));
+  }
+
+  return NULL;
+}
+
 EXTERN_C_START
 napi_value Init(napi_env env, napi_value exports) {
   napi_property_descriptor properties[] = {
@@ -258,6 +277,7 @@ napi_value Init(napi_env env, napi_value exports) {
     DECLARE_NAPI_PROPERTY("TestLargeUtf8", TestLargeUtf8),
     DECLARE_NAPI_PROPERTY("TestLargeLatin1", TestLargeLatin1),
     DECLARE_NAPI_PROPERTY("TestLargeUtf16", TestLargeUtf16),
+    DECLARE_NAPI_PROPERTY("TestMemoryCorruption", TestMemoryCorruption),
   };
 
   NAPI_CALL(env, napi_define_properties(