fix: crash on windows when UTF-8 is in path (#48947)

In 6399527761b43abb5a3ea40dd64eb05efad3b9de we changed the path strings
that `node_modules.cc` operates on from single-byte to wide strings.
Unfortunately this means that `generic_path()` that the
"fix: ensure TraverseParent bails on resource path exit" patch was
calling was no longer a safe method to call on Windows if the underlying
string has unicode characters in it.

Here we fix it by using `ConvertGenericPathToUTF8` from the Node.js
internal utilities.
This commit is contained in:
Fedor Indutny 2025-11-13 14:49:48 -08:00 committed by GitHub
commit 40d65d5a9f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 15 additions and 19 deletions

View file

@ -39,7 +39,6 @@ build_change_crdtp_protocoltypetraits_signatures_to_avoid_conflict.patch
fix_adjust_wpt_and_webidl_tests_for_enabled_float16array.patch fix_adjust_wpt_and_webidl_tests_for_enabled_float16array.patch
chore_add_createexternalizabletwobytestring_to_globals.patch chore_add_createexternalizabletwobytestring_to_globals.patch
refactor_attach_cppgc_heap_on_v8_isolate_creation.patch refactor_attach_cppgc_heap_on_v8_isolate_creation.patch
fix_ensure_traverseparent_bails_on_resource_path_exit.patch
cli_move_--trace-atomics-wait_to_eol.patch cli_move_--trace-atomics-wait_to_eol.patch
fix_cppgc_initializing_twice.patch fix_cppgc_initializing_twice.patch
fix_task_starvation_in_inspector_context_test.patch fix_task_starvation_in_inspector_context_test.patch
@ -47,3 +46,4 @@ fix_expose_readfilesync_override_for_modules.patch
fix_array_out-of-bounds_read_in_boyer-moore_search.patch fix_array_out-of-bounds_read_in_boyer-moore_search.patch
chore_add_missing_include_of_iterator.patch chore_add_missing_include_of_iterator.patch
src_use_cp_utf8_for_wide_file_names_on_win32.patch src_use_cp_utf8_for_wide_file_names_on_win32.patch
fix_ensure_traverseparent_bails_on_resource_path_exit.patch

View file

@ -8,10 +8,10 @@ resource path. This commit ensures that the TraverseParent function
bails out if the parent path is outside of the resource path. bails out if the parent path is outside of the resource path.
diff --git a/src/node_modules.cc b/src/node_modules.cc diff --git a/src/node_modules.cc b/src/node_modules.cc
index 60b03b1563b230f78d8a9bdd8480da4d2ae90a27..35bfada261258407982d9e24cf7b3e820235c941 100644 index 0e820f8623ecf41008a938f16d11bd3206a7b0c1..6e06717cd4e50a8c2a1d91d14a23b7332e817bdd 100644
--- a/src/node_modules.cc --- a/src/node_modules.cc
+++ b/src/node_modules.cc +++ b/src/node_modules.cc
@@ -279,8 +279,41 @@ const BindingData::PackageConfig* BindingData::TraverseParent( @@ -315,8 +315,41 @@ const BindingData::PackageConfig* BindingData::TraverseParent(
Realm* realm, const std::filesystem::path& check_path) { Realm* realm, const std::filesystem::path& check_path) {
std::filesystem::path current_path = check_path; std::filesystem::path current_path = check_path;
auto env = realm->env(); auto env = realm->env();
@ -47,22 +47,22 @@ index 60b03b1563b230f78d8a9bdd8480da4d2ae90a27..35bfada261258407982d9e24cf7b3e82
+ }); + });
+ }; + };
+ +
+ bool did_original_path_start_with_resources_path = starts_with(check_path. + bool did_original_path_start_with_resources_path = starts_with(
+ generic_string(), resources_path); + ConvertGenericPathToUTF8(check_path), resources_path);
+ +
do { do {
current_path = current_path.parent_path(); current_path = current_path.parent_path();
@@ -299,6 +332,12 @@ const BindingData::PackageConfig* BindingData::TraverseParent( @@ -336,6 +369,12 @@ const BindingData::PackageConfig* BindingData::TraverseParent(
return nullptr; }
} }
+ // If current path is outside the resources path, bail. + // If current path is outside the resources path, bail.
+ if (did_original_path_start_with_resources_path && + if (did_original_path_start_with_resources_path &&
+ !starts_with(current_path.generic_string(), resources_path)) { + !starts_with(ConvertGenericPathToUTF8(current_path), resources_path)) {
+ return nullptr; + return nullptr;
+ } + }
+ +
// Check if the path ends with `/node_modules` // Check if the path ends with `/node_modules`
if (current_path.generic_string().ends_with("/node_modules")) { if (current_path.filename() == "node_modules") {
return nullptr; return nullptr;

View file

@ -20,7 +20,7 @@ index 82225b0a53dd828750991a4e15a060b736b6ea2b..4b0d31356a2496a7fc67876a22da2453
V(performance_entry_callback, v8::Function) \ V(performance_entry_callback, v8::Function) \
V(prepare_stack_trace_callback, v8::Function) \ V(prepare_stack_trace_callback, v8::Function) \
diff --git a/src/node_modules.cc b/src/node_modules.cc diff --git a/src/node_modules.cc b/src/node_modules.cc
index 35bfada261258407982d9e24cf7b3e820235c941..ed22da844a61b14b8580cd3d6bb3a233b8559b38 100644 index 60b03b1563b230f78d8a9bdd8480da4d2ae90a27..62050791174563f88b8629d576eed8959b3c2e20 100644
--- a/src/node_modules.cc --- a/src/node_modules.cc
+++ b/src/node_modules.cc +++ b/src/node_modules.cc
@@ -21,6 +21,7 @@ namespace modules { @@ -21,6 +21,7 @@ namespace modules {
@ -90,7 +90,7 @@ index 35bfada261258407982d9e24cf7b3e820235c941..ed22da844a61b14b8580cd3d6bb3a233
void BindingData::ReadPackageJSON(const FunctionCallbackInfo<Value>& args) { void BindingData::ReadPackageJSON(const FunctionCallbackInfo<Value>& args) {
CHECK_GE(args.Length(), 1); // path, [is_esm, base, specifier] CHECK_GE(args.Length(), 1); // path, [is_esm, base, specifier]
CHECK(args[0]->IsString()); // path CHECK(args[0]->IsString()); // path
@@ -597,6 +633,8 @@ void InitImportMetaPathHelpers(const FunctionCallbackInfo<Value>& args) { @@ -558,6 +594,8 @@ void InitImportMetaPathHelpers(const FunctionCallbackInfo<Value>& args) {
void BindingData::CreatePerIsolateProperties(IsolateData* isolate_data, void BindingData::CreatePerIsolateProperties(IsolateData* isolate_data,
Local<ObjectTemplate> target) { Local<ObjectTemplate> target) {
Isolate* isolate = isolate_data->isolate(); Isolate* isolate = isolate_data->isolate();
@ -99,7 +99,7 @@ index 35bfada261258407982d9e24cf7b3e820235c941..ed22da844a61b14b8580cd3d6bb3a233
SetMethod(isolate, target, "readPackageJSON", ReadPackageJSON); SetMethod(isolate, target, "readPackageJSON", ReadPackageJSON);
SetMethod(isolate, SetMethod(isolate,
target, target,
@@ -635,6 +673,8 @@ void BindingData::CreatePerContextProperties(Local<Object> target, @@ -596,6 +634,8 @@ void BindingData::CreatePerContextProperties(Local<Object> target,
void BindingData::RegisterExternalReferences( void BindingData::RegisterExternalReferences(
ExternalReferenceRegistry* registry) { ExternalReferenceRegistry* registry) {

View file

@ -136,10 +136,10 @@ index 5de3ebb04b12286a07e3041d0a6dd1cc9072e76a..52075b7d21c7c4674ccd81698ea5595c
std::make_error_code(std::errc::file_exists), std::make_error_code(std::errc::file_exists),
"cp", "cp",
diff --git a/src/node_modules.cc b/src/node_modules.cc diff --git a/src/node_modules.cc b/src/node_modules.cc
index ed22da844a61b14b8580cd3d6bb3a233b8559b38..2c5c14f506f39abefe3c8c802862264aef3a4fe4 100644 index 62050791174563f88b8629d576eed8959b3c2e20..0e820f8623ecf41008a938f16d11bd3206a7b0c1 100644
--- a/src/node_modules.cc --- a/src/node_modules.cc
+++ b/src/node_modules.cc +++ b/src/node_modules.cc
@@ -360,12 +360,13 @@ const BindingData::PackageConfig* BindingData::TraverseParent( @@ -327,22 +327,24 @@ const BindingData::PackageConfig* BindingData::TraverseParent(
// Stop the search when the process doesn't have permissions // Stop the search when the process doesn't have permissions
// to walk upwards // to walk upwards
@ -158,10 +158,6 @@ index ed22da844a61b14b8580cd3d6bb3a233b8559b38..2c5c14f506f39abefe3c8c802862264a
+ } + }
} }
// If current path is outside the resources path, bail.
@@ -375,13 +376,14 @@ const BindingData::PackageConfig* BindingData::TraverseParent(
}
// Check if the path ends with `/node_modules` // Check if the path ends with `/node_modules`
- if (current_path.generic_string().ends_with("/node_modules")) { - if (current_path.generic_string().ends_with("/node_modules")) {
+ if (current_path.filename() == "node_modules") { + if (current_path.filename() == "node_modules") {
@ -176,7 +172,7 @@ index ed22da844a61b14b8580cd3d6bb3a233b8559b38..2c5c14f506f39abefe3c8c802862264a
if (package_json != nullptr) { if (package_json != nullptr) {
return package_json; return package_json;
} }
@@ -403,20 +405,12 @@ void BindingData::GetNearestParentPackageJSONType( @@ -364,20 +366,12 @@ void BindingData::GetNearestParentPackageJSONType(
ToNamespacedPath(realm->env(), &path_value); ToNamespacedPath(realm->env(), &path_value);