Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit f7724ab

Browse filesBrowse files
gabrielschulhofdanielleadams
authored andcommitted
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>
1 parent 8c7b2ba commit f7724ab
Copy full SHA for f7724ab

File tree

Expand file treeCollapse file tree

6 files changed

+108
-1
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

6 files changed

+108
-1
lines changed
Open diff view settings
Collapse file

‎src/js_native_api_v8.cc‎

Copy file name to clipboardExpand all lines: src/js_native_api_v8.cc
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1485,6 +1485,8 @@ napi_status napi_create_string_latin1(napi_env env,
14851485
size_t length,
14861486
napi_value* result) {
14871487
CHECK_ENV(env);
1488+
if (length > 0)
1489+
CHECK_ARG(env, str);
14881490
CHECK_ARG(env, result);
14891491
RETURN_STATUS_IF_FALSE(env,
14901492
(length == NAPI_AUTO_LENGTH) || length <= INT_MAX,
@@ -1507,6 +1509,8 @@ napi_status napi_create_string_utf8(napi_env env,
15071509
size_t length,
15081510
napi_value* result) {
15091511
CHECK_ENV(env);
1512+
if (length > 0)
1513+
CHECK_ARG(env, str);
15101514
CHECK_ARG(env, result);
15111515
RETURN_STATUS_IF_FALSE(env,
15121516
(length == NAPI_AUTO_LENGTH) || length <= INT_MAX,
@@ -1528,6 +1532,8 @@ napi_status napi_create_string_utf16(napi_env env,
15281532
size_t length,
15291533
napi_value* result) {
15301534
CHECK_ENV(env);
1535+
if (length > 0)
1536+
CHECK_ARG(env, str);
15311537
CHECK_ARG(env, result);
15321538
RETURN_STATUS_IF_FALSE(env,
15331539
(length == NAPI_AUTO_LENGTH) || length <= INT_MAX,
Collapse file

‎test/js-native-api/test_string/binding.gyp‎

Copy file name to clipboardExpand all lines: test/js-native-api/test_string/binding.gyp
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
"target_name": "test_string",
55
"sources": [
66
"../entry_point.c",
7-
"test_string.c"
7+
"test_string.c",
8+
"test_null.c",
9+
"../common.c",
810
]
911
}
1012
]
Collapse file
+71Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
#include <js_native_api.h>
2+
3+
#include "../common.h"
4+
#include "test_null.h"
5+
6+
#define DECLARE_TEST(charset, str_arg) \
7+
static napi_value \
8+
test_create_##charset(napi_env env, napi_callback_info info) { \
9+
napi_value return_value, result; \
10+
NODE_API_CALL(env, napi_create_object(env, &return_value)); \
11+
\
12+
add_returned_status(env, \
13+
"envIsNull", \
14+
return_value, \
15+
"Invalid argument", \
16+
napi_invalid_arg, \
17+
napi_create_string_##charset(NULL, \
18+
(str_arg), \
19+
NAPI_AUTO_LENGTH, \
20+
&result)); \
21+
\
22+
napi_create_string_##charset(env, NULL, NAPI_AUTO_LENGTH, &result); \
23+
add_last_status(env, "stringIsNullNonZeroLength", return_value); \
24+
\
25+
napi_create_string_##charset(env, NULL, 0, &result); \
26+
add_last_status(env, "stringIsNullZeroLength", return_value); \
27+
\
28+
napi_create_string_##charset(env, (str_arg), NAPI_AUTO_LENGTH, NULL); \
29+
add_last_status(env, "resultIsNull", return_value); \
30+
\
31+
return return_value; \
32+
}
33+
34+
static const char16_t something[] = {
35+
(char16_t)'s',
36+
(char16_t)'o',
37+
(char16_t)'m',
38+
(char16_t)'e',
39+
(char16_t)'t',
40+
(char16_t)'h',
41+
(char16_t)'i',
42+
(char16_t)'n',
43+
(char16_t)'g',
44+
(char16_t)'\0'
45+
};
46+
47+
DECLARE_TEST(utf8, "something")
48+
DECLARE_TEST(latin1, "something")
49+
DECLARE_TEST(utf16, something)
50+
51+
void init_test_null(napi_env env, napi_value exports) {
52+
napi_value test_null;
53+
54+
const napi_property_descriptor test_null_props[] = {
55+
DECLARE_NODE_API_PROPERTY("test_create_utf8", test_create_utf8),
56+
DECLARE_NODE_API_PROPERTY("test_create_latin1", test_create_latin1),
57+
DECLARE_NODE_API_PROPERTY("test_create_utf16", test_create_utf16),
58+
};
59+
60+
NODE_API_CALL_RETURN_VOID(env, napi_create_object(env, &test_null));
61+
NODE_API_CALL_RETURN_VOID(env, napi_define_properties(
62+
env, test_null, sizeof(test_null_props) / sizeof(*test_null_props),
63+
test_null_props));
64+
65+
const napi_property_descriptor test_null_set = {
66+
"testNull", NULL, NULL, NULL, NULL, test_null, napi_enumerable, NULL
67+
};
68+
69+
NODE_API_CALL_RETURN_VOID(env,
70+
napi_define_properties(env, exports, 1, &test_null_set));
71+
}
Collapse file
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
#ifndef TEST_JS_NATIVE_API_TEST_STRING_TEST_NULL_H_
2+
#define TEST_JS_NATIVE_API_TEST_STRING_TEST_NULL_H_
3+
4+
#include <js_native_api.h>
5+
6+
void init_test_null(napi_env env, napi_value exports);
7+
8+
#endif // TEST_JS_NATIVE_API_TEST_STRING_TEST_NULL_H_
Collapse file
+17Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
'use strict';
2+
const common = require('../../common');
3+
const assert = require('assert');
4+
5+
// Test passing NULL to object-related N-APIs.
6+
const { testNull } = require(`./build/${common.buildType}/test_string`);
7+
8+
const expectedResult = {
9+
envIsNull: 'Invalid argument',
10+
stringIsNullNonZeroLength: 'Invalid argument',
11+
stringIsNullZeroLength: 'napi_ok',
12+
resultIsNull: 'Invalid argument',
13+
};
14+
15+
assert.deepStrictEqual(expectedResult, testNull.test_create_latin1());
16+
assert.deepStrictEqual(expectedResult, testNull.test_create_utf8());
17+
assert.deepStrictEqual(expectedResult, testNull.test_create_utf16());
Collapse file

‎test/js-native-api/test_string/test_string.c‎

Copy file name to clipboardExpand all lines: test/js-native-api/test_string/test_string.c
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#include <string.h>
33
#include <js_native_api.h>
44
#include "../common.h"
5+
#include "test_null.h"
56

67
static napi_value TestLatin1(napi_env env, napi_callback_info info) {
78
size_t argc = 1;
@@ -283,6 +284,8 @@ napi_value Init(napi_env env, napi_value exports) {
283284
DECLARE_NODE_API_PROPERTY("TestMemoryCorruption", TestMemoryCorruption),
284285
};
285286

287+
init_test_null(env, exports);
288+
286289
NODE_API_CALL(env, napi_define_properties(
287290
env, exports, sizeof(properties) / sizeof(*properties), properties));
288291

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.