From b5dc71755f035e7aaac9823f0a8fba0ef8f326d4 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Thu, 3 Jun 2021 23:05:07 -0700 Subject: [PATCH] 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 --- src/js_native_api_v8.cc | 3 + test/js-native-api/test_string/binding.gyp | 4 +- test/js-native-api/test_string/test_null.c | 68 ++++++++++++++++++++ test/js-native-api/test_string/test_null.h | 8 +++ test/js-native-api/test_string/test_null.js | 16 +++++ test/js-native-api/test_string/test_string.c | 3 + 6 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 test/js-native-api/test_string/test_null.c create mode 100644 test/js-native-api/test_string/test_null.h create mode 100644 test/js-native-api/test_string/test_null.js diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 2aeb96a17a599a..c92dfff8fc9ec7 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -1483,6 +1483,7 @@ napi_status napi_create_string_latin1(napi_env env, size_t length, napi_value* result) { CHECK_ENV(env); + CHECK_ARG(env, str); CHECK_ARG(env, result); RETURN_STATUS_IF_FALSE(env, (length == NAPI_AUTO_LENGTH) || length <= INT_MAX, @@ -1505,6 +1506,7 @@ napi_status napi_create_string_utf8(napi_env env, size_t length, napi_value* result) { CHECK_ENV(env); + CHECK_ARG(env, str); CHECK_ARG(env, result); RETURN_STATUS_IF_FALSE(env, (length == NAPI_AUTO_LENGTH) || length <= INT_MAX, @@ -1526,6 +1528,7 @@ napi_status napi_create_string_utf16(napi_env env, size_t length, napi_value* result) { CHECK_ENV(env); + 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 index 8b0f3e33543d39..c2f55857d41fe7 100644 --- 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 new file mode 100644 index 00000000000000..42455defc31c9d --- /dev/null +++ b/test/js-native-api/test_string/test_null.c @@ -0,0 +1,68 @@ +#include + +#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, "stringIsNull", 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 new file mode 100644 index 00000000000000..fdeb17384b4f0b --- /dev/null +++ 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 + +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 new file mode 100644 index 00000000000000..8dcd41b9f403d6 --- /dev/null +++ b/test/js-native-api/test_string/test_null.js @@ -0,0 +1,16 @@ +'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', + stringIsNull: 'Invalid argument', + 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 index 1dc1bf75774472..c78d761fb2ee59 100644 --- 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 #include #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));