Skip to content

Commit 77b8e56

Browse files
authored
fix update_base_pathname() creating invalid state (#1077)
1 parent 4decd01 commit 77b8e56

File tree

3 files changed

+81
-0
lines changed

3 files changed

+81
-0
lines changed

include/ada/url_aggregator-inl.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,12 @@ inline void url_aggregator::update_base_pathname(const std::string_view input) {
278278
// output.
279279
buffer.insert(components.pathname_start, "/.");
280280
components.pathname_start += 2;
281+
if (components.search_start != url_components::omitted) {
282+
components.search_start += 2;
283+
}
284+
if (components.hash_start != url_components::omitted) {
285+
components.hash_start += 2;
286+
}
281287
}
282288

283289
uint32_t difference = replace_and_resize(

src/url_aggregator.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,12 @@ bool url_aggregator::set_pathname(const std::string_view input) {
329329
if (get_pathname().starts_with("//") && !has_authority() && !has_dash_dot()) {
330330
buffer.insert(components.pathname_start, "/.");
331331
components.pathname_start += 2;
332+
if (components.search_start != url_components::omitted) {
333+
components.search_start += 2;
334+
}
335+
if (components.hash_start != url_components::omitted) {
336+
components.hash_start += 2;
337+
}
332338
}
333339
ADA_ASSERT_TRUE(validate());
334340
return true;

tests/basic_tests.cpp

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,3 +568,72 @@ TYPED_TEST(basic_tests, test_workerd_issue_5144_4) {
568568
ASSERT_TRUE(pattern->exec(std::move(dummy_init)));
569569
SUCCEED();
570570
}
571+
572+
// https://github.com/ada-url/ada/issues/1076
573+
// Setting pathname to a "//" path on a non-special URL without authority but
574+
// with a query or hash component should not trigger a validate() assertion
575+
// failure caused by stale search_start/hash_start offsets after "/." insertion.
576+
TEST(basic_tests, issue_1076_set_pathname_dashdot_with_query) {
577+
// Non-special URL with a query: no authority, has search component
578+
auto url = ada::parse<ada::url_aggregator>("foo:/?q");
579+
ASSERT_TRUE(url);
580+
ASSERT_TRUE(url->validate());
581+
ASSERT_TRUE(url->set_pathname("//bar"));
582+
ASSERT_TRUE(url->validate());
583+
ASSERT_EQ(url->get_pathname(), "//bar");
584+
ASSERT_EQ(url->get_search(), "?q");
585+
SUCCEED();
586+
}
587+
588+
TEST(basic_tests, issue_1076_set_pathname_dashdot_with_hash) {
589+
// Non-special URL with a hash: no authority, has hash component
590+
auto url = ada::parse<ada::url_aggregator>("foo:/#h");
591+
ASSERT_TRUE(url);
592+
ASSERT_TRUE(url->validate());
593+
ASSERT_TRUE(url->set_pathname("//bar"));
594+
ASSERT_TRUE(url->validate());
595+
ASSERT_EQ(url->get_pathname(), "//bar");
596+
ASSERT_EQ(url->get_hash(), "#h");
597+
SUCCEED();
598+
}
599+
600+
TEST(basic_tests, issue_1076_set_pathname_dashdot_with_query_and_hash) {
601+
// Non-special URL with both query and hash
602+
auto url = ada::parse<ada::url_aggregator>("foo:/?q#h");
603+
ASSERT_TRUE(url);
604+
ASSERT_TRUE(url->validate());
605+
ASSERT_TRUE(url->set_pathname("//bar"));
606+
ASSERT_TRUE(url->validate());
607+
ASSERT_EQ(url->get_pathname(), "//bar");
608+
ASSERT_EQ(url->get_search(), "?q");
609+
ASSERT_EQ(url->get_hash(), "#h");
610+
SUCCEED();
611+
}
612+
613+
TEST(basic_tests, issue_1076_blob_with_query) {
614+
// blob: scheme with query - similar to existing path_setter_bug test
615+
auto url = ada::parse<ada::url_aggregator>("blob:/?q");
616+
ASSERT_TRUE(url);
617+
ASSERT_TRUE(url->validate());
618+
ASSERT_TRUE(url->set_pathname("//p"));
619+
ASSERT_TRUE(url->validate());
620+
ASSERT_EQ(url->get_pathname(), "//p");
621+
ASSERT_EQ(url->get_search(), "?q");
622+
SUCCEED();
623+
}
624+
625+
TEST(basic_tests, issue_1076_setter_sequence) {
626+
// Simulates the fuzzer scenario: parse a URL, then call multiple setters
627+
// to put it into a vulnerable state before set_pathname
628+
auto url = ada::parse<ada::url_aggregator>("foo://host/path?query#hash");
629+
ASSERT_TRUE(url);
630+
ASSERT_TRUE(url->validate());
631+
// Clear the host to remove authority
632+
url->set_hostname("");
633+
url->set_host("");
634+
ASSERT_TRUE(url->validate());
635+
// Now set pathname to something starting with //
636+
ASSERT_TRUE(url->set_pathname("//newpath"));
637+
ASSERT_TRUE(url->validate());
638+
SUCCEED();
639+
}

0 commit comments

Comments
 (0)