node-api: avoid crashing on passed-in null string

When `napi_create_string_*` receives a null pointer as its second argument, it must null-check it before passing it into V8, otherwise a crash will occur. Signed-off-by: Gabriel Schulhof <gabrielschulhof@gmail.com> PR-URL: #38923 Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
nodejs · Jun 13, 2021 · f7724ab · f7724ab
1 parent 8c7b2ba
commit f7724ab
Show file tree

Hide file tree

Showing 6 changed files with 108 additions and 1 deletion.
diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc
@@ -1485,6 +1485,8 @@ napi_status napi_create_string_latin1(napi_env env,
                                       size_t length,
                                       napi_value* result) {
   CHECK_ENV(env);
+  if (length > 0)
+      CHECK_ARG(env, str);
   CHECK_ARG(env, result);
   RETURN_STATUS_IF_FALSE(env,
       (length == NAPI_AUTO_LENGTH) || length <= INT_MAX,
@@ -1507,6 +1509,8 @@ napi_status napi_create_string_utf8(napi_env env,
                                     size_t length,
                                     napi_value* result) {
   CHECK_ENV(env);
+  if (length > 0)
+      CHECK_ARG(env, str);
   CHECK_ARG(env, result);
   RETURN_STATUS_IF_FALSE(env,
       (length == NAPI_AUTO_LENGTH) || length <= INT_MAX,
@@ -1528,6 +1532,8 @@ napi_status napi_create_string_utf16(napi_env env,
                                      size_t length,
                                      napi_value* result) {
   CHECK_ENV(env);
+  if (length > 0)
+      CHECK_ARG(env, str);
   CHECK_ARG(env, result);
   RETURN_STATUS_IF_FALSE(env,
       (length == NAPI_AUTO_LENGTH) || length <= INT_MAX,

diff --git a/test/js-native-api/test_string/binding.gyp b/test/js-native-api/test_string/binding.gyp
@@ -4,7 +4,9 @@
       "target_name": "test_string",
       "sources": [
         "../entry_point.c",
-        "test_string.c"
+        "test_string.c",
+        "test_null.c",
+        "../common.c",
       ]
     }
   ]

diff --git a/test/js-native-api/test_string/test_null.c b/test/js-native-api/test_string/test_null.c
@@ -0,0 +1,71 @@
+#include <js_native_api.h>
+
+#include "../common.h"
+#include "test_null.h"
+
+#define DECLARE_TEST(charset, str_arg)                                    \
+  static napi_value                                                       \
+  test_create_##charset(napi_env env, napi_callback_info info) {          \
+    napi_value return_value, result;                                      \
+    NODE_API_CALL(env, napi_create_object(env, &return_value));           \
+                                                                          \
+    add_returned_status(env,                                              \
+                        "envIsNull",                                      \
+                        return_value,                                     \
+                        "Invalid argument",                               \
+                        napi_invalid_arg,                                 \
+                        napi_create_string_##charset(NULL,                \
+                                                     (str_arg),           \
+                                                     NAPI_AUTO_LENGTH,    \
+                                                     &result));           \
+                                                                          \
+    napi_create_string_##charset(env, NULL, NAPI_AUTO_LENGTH, &result);   \
+    add_last_status(env, "stringIsNullNonZeroLength", return_value);      \
+                                                                          \
+    napi_create_string_##charset(env, NULL, 0, &result);                  \
+    add_last_status(env, "stringIsNullZeroLength", return_value);         \
+                                                                          \
+    napi_create_string_##charset(env, (str_arg), NAPI_AUTO_LENGTH, NULL); \
+    add_last_status(env, "resultIsNull", return_value);                   \
+                                                                          \
+    return return_value;                                                  \
+  }
+
+static const char16_t something[] = {
+  (char16_t)'s',
+  (char16_t)'o',
+  (char16_t)'m',
+  (char16_t)'e',
+  (char16_t)'t',
+  (char16_t)'h',
+  (char16_t)'i',
+  (char16_t)'n',
+  (char16_t)'g',
+  (char16_t)'\0'
+};
+
+DECLARE_TEST(utf8, "something")
+DECLARE_TEST(latin1, "something")
+DECLARE_TEST(utf16, something)
+
+void init_test_null(napi_env env, napi_value exports) {
+  napi_value test_null;
+
+  const napi_property_descriptor test_null_props[] = {
+    DECLARE_NODE_API_PROPERTY("test_create_utf8", test_create_utf8),
+    DECLARE_NODE_API_PROPERTY("test_create_latin1", test_create_latin1),
+    DECLARE_NODE_API_PROPERTY("test_create_utf16", test_create_utf16),
+  };
+
+  NODE_API_CALL_RETURN_VOID(env, napi_create_object(env, &test_null));
+  NODE_API_CALL_RETURN_VOID(env, napi_define_properties(
+      env, test_null, sizeof(test_null_props) / sizeof(*test_null_props),
+      test_null_props));
+
+  const napi_property_descriptor test_null_set = {
+    "testNull", NULL, NULL, NULL, NULL, test_null, napi_enumerable, NULL
+  };
+
+  NODE_API_CALL_RETURN_VOID(env,
+      napi_define_properties(env, exports, 1, &test_null_set));
+}
diff --git a/test/js-native-api/test_string/test_null.h b/test/js-native-api/test_string/test_null.h
@@ -0,0 +1,8 @@
+#ifndef TEST_JS_NATIVE_API_TEST_STRING_TEST_NULL_H_
+#define TEST_JS_NATIVE_API_TEST_STRING_TEST_NULL_H_
+
+#include <js_native_api.h>
+
+void init_test_null(napi_env env, napi_value exports);
+
+#endif  // TEST_JS_NATIVE_API_TEST_STRING_TEST_NULL_H_
diff --git a/test/js-native-api/test_string/test_null.js b/test/js-native-api/test_string/test_null.js
@@ -0,0 +1,17 @@
+'use strict';
+const common = require('../../common');
+const assert = require('assert');
+
+// Test passing NULL to object-related N-APIs.
+const { testNull } = require(`./build/${common.buildType}/test_string`);
+
+const expectedResult = {
+  envIsNull: 'Invalid argument',
+  stringIsNullNonZeroLength: 'Invalid argument',
+  stringIsNullZeroLength: 'napi_ok',
+  resultIsNull: 'Invalid argument',
+};
+
+assert.deepStrictEqual(expectedResult, testNull.test_create_latin1());
+assert.deepStrictEqual(expectedResult, testNull.test_create_utf8());
+assert.deepStrictEqual(expectedResult, testNull.test_create_utf16());
diff --git a/test/js-native-api/test_string/test_string.c b/test/js-native-api/test_string/test_string.c
@@ -2,6 +2,7 @@
 #include <string.h>
 #include <js_native_api.h>
 #include "../common.h"
+#include "test_null.h"
 
 static napi_value TestLatin1(napi_env env, napi_callback_info info) {
   size_t argc = 1;
@@ -283,6 +284,8 @@ napi_value Init(napi_env env, napi_value exports) {
     DECLARE_NODE_API_PROPERTY("TestMemoryCorruption", TestMemoryCorruption),
   };
 
+  init_test_null(env, exports);
+
   NODE_API_CALL(env, napi_define_properties(
       env, exports, sizeof(properties) / sizeof(*properties), properties));