Commit 001c37c
url: bounds-check short Windows file URL paths
Check the decoded pathname length before reading the drive letter and
colon. This prevents an out-of-bounds read for short URLs such as
file:/// and reports ERR_INVALID_FILE_URL_PATH instead.
Signed-off-by: Archkon <180910180+Archkon@users.noreply.github.com>
PR-URL: #64788
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
Reviewed-By: Aviv Keller <me@aviv.sh>1 parent b5182f0 commit 001c37c
2 files changed
Lines changed: 30 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -665,6 +665,12 @@ std::optional<std::string> FileURLToPath(Environment* env, | |||
| 665 | 665 | return "\\\\" + ada::idna::to_unicode(hostname) + decoded_pathname; | |
| 666 | 666 | } | |
| 667 | 667 | ||
| 668 | + if (decoded_pathname.size() < 3) { | ||
| 669 | + THROW_ERR_INVALID_FILE_URL_PATH(env->isolate(), | ||
| 670 | + "File URL path must be absolute"); | ||
| 671 | + return std::nullopt; | ||
| 672 | + } | ||
| 673 | + | ||
| 668 | 674 | char letter = decoded_pathname[1] | 0x20; | |
| 669 | 675 | char sep = decoded_pathname[2]; | |
| 670 | 676 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -8,6 +8,7 @@ | |||
| 8 | 8 | #include "v8.h" | |
| 9 | 9 | ||
| 10 | 10 | using node::BufferValue; | |
| 11 | + using node::NormalizeFileURLOrPath; | ||
| 11 | 12 | using node::PathResolve; | |
| 12 | 13 | using node::ToNamespacedPath; | |
| 13 | 14 | ||
@@ -93,3 +94,26 @@ TEST_F(PathTest, ToNamespacedPath) { | |||
| 93 | 94 | EXPECT_EQ(data.ToStringView(), "hello world"); // Input should not be mutated | |
| 94 | 95 | #endif | |
| 95 | 96 | } | |
| 97 | + | ||
| 98 | + #ifdef _WIN32 | ||
| 99 | + TEST_F(PathTest, NormalizeShortFileURLPath) { | ||
| 100 | + const v8::HandleScope handle_scope(isolate_); | ||
| 101 | + Argv argv; | ||
| 102 | + Env env{handle_scope, argv, node::EnvironmentFlags::kNoBrowserGlobals}; | ||
| 103 | + v8::TryCatch try_catch(isolate_); | ||
| 104 | + | ||
| 105 | + EXPECT_EQ(NormalizeFileURLOrPath(*env, "file:///"), ""); | ||
| 106 | + ASSERT_TRUE(try_catch.HasCaught()); | ||
| 107 | + | ||
| 108 | + v8::Local<v8::Value> exception = try_catch.Exception(); | ||
| 109 | + ASSERT_TRUE(exception->IsObject()); | ||
| 110 | + v8::Local<v8::Value> code; | ||
| 111 | + ASSERT_TRUE(exception.As<v8::Object>() | ||
| 112 | + ->Get((*env)->context(), | ||
| 113 | + v8::String::NewFromUtf8Literal(isolate_, "code")) | ||
| 114 | + .ToLocal(&code)); | ||
| 115 | + ASSERT_TRUE(code->IsString()); | ||
| 116 | + node::Utf8Value code_value(isolate_, code); | ||
| 117 | + EXPECT_EQ(code_value.ToStringView(), "ERR_INVALID_FILE_URL_PATH"); | ||
| 118 | + } | ||
| 119 | + #endif | ||
0 commit comments