Skip to content

Commit

Permalink
fix: libc++ buffer overflow in string_view ctor
Browse files Browse the repository at this point in the history
  • Loading branch information
codebytere committed Apr 18, 2023
1 parent dd0e744 commit 3e0eb41
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 0 deletions.
2 changes: 2 additions & 0 deletions patches/node/.patches
Expand Up @@ -31,3 +31,5 @@ fix_prevent_changing_functiontemplateinfo_after_publish.patch
enable_crashpad_linux_node_processes.patch
test_mark_cpu_prof_tests_as_flaky_in_electron.patch
fix_adapt_debugger_tests_for_upstream_v8_changes.patch
fix_libc_buffer_overflow_in_string_view_ctor.patch
fix_preventing_potential_oob_in_ada_no_scheme_parsing.patch
40 changes: 40 additions & 0 deletions patches/node/fix_libc_buffer_overflow_in_string_view_ctor.patch
@@ -0,0 +1,40 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Shelley Vohr <shelley.vohr@gmail.com>
Date: Tue, 18 Apr 2023 11:24:59 +0200
Subject: fix: libc++ buffer overflow in string_view ctor

Refs https://github.com/nodejs/node/pull/46410
Refs https://github.com/llvm/llvm-project/issues/61100.

Fixes a buffer overflow crash in std::string view constructor. This
happens as a result of a limitation of libc++'s implementation of the
string_view constructor. If string_view receives a pointer to a string start point,
and then a length as a size_t, the spec says that a size_t that exceeds the length
of the null terminated string found at pointer only be read up until the null terminator.

However, Chromium's implementation however does a hard check that this length is less than
or equal to static_cast<size_t>(std::numeric_limits<std::ptrdiff_t>::max()):
https://source.chromium.org/chromium/chromium/src/+/main:buildtools/third_party/libc++/trunk/include/string_view;drc=1718a75513d114e6b9aa4474e5c55d8dabee92fb;l=309

This doesn't break in Node.js right now because that hard assert is missing, but will
break in the next version of Xcode that's shipped and this Node.js too.

This should be upstreamed to ada.

diff --git a/deps/ada/ada.cpp b/deps/ada/ada.cpp
index 8b2cdd38ad0bb1e5757cdf5724c5a5917fc8e577..7d2f75fe559c1878e129aa681c86d6780d2e8233 100644
--- a/deps/ada/ada.cpp
+++ b/deps/ada/ada.cpp
@@ -825,8 +825,10 @@ namespace ada::helpers {

// Let path be url’s path.
// If url’s scheme is "file", path’s size is 1, and path[0] is a normalized Windows drive letter, then return.
- if (type == ada::scheme::type::FILE && first_delimiter == std::string_view::npos) {
- if (checkers::is_normalized_windows_drive_letter(std::string_view(path.data() + 1, first_delimiter - 1))) {
+ if (!path.empty() && type == ada::scheme::type::FILE &&
+ first_delimiter == std::string_view::npos) {
+ if (checkers::is_normalized_windows_drive_letter(
+ std::string_view(path.data() + 1, path.length() - 1))) {
return;
}
}
@@ -0,0 +1,21 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Shelley Vohr <shelley.vohr@gmail.com>
Date: Tue, 18 Apr 2023 15:08:05 +0200
Subject: fix: preventing potential oob in ada NO_SCHEME parsing

https://github.com/ada-url/ada/pull/286

diff --git a/deps/ada/ada.cpp b/deps/ada/ada.cpp
index 7d2f75fe559c1878e129aa681c86d6780d2e8233..7bf541834d22511eefcc58517cc75117f53eb475 100644
--- a/deps/ada/ada.cpp
+++ b/deps/ada/ada.cpp
@@ -2243,7 +2243,8 @@ namespace ada::parser {
ada_log("NO_SCHEME ", helpers::substring(url_data, input_position));
// If base is null, or base has an opaque path and c is not U+0023 (#), validation error, return failure.
// SCHEME state updates the state to NO_SCHEME and validates url_data is not empty.
- if (base_url == nullptr || (base_url->has_opaque_path && url_data[input_position] != '#')) {
+ if (base_url == nullptr ||
+ (base_url->has_opaque_path && !fragment.has_value())) {
ada_log("NO_SCHEME validation error");
url.is_valid = false;
return url;

0 comments on commit 3e0eb41

Please sign in to comment.