Ladybird/ladybird
1
0
Fork 0
mirror of https://github.com/LadybirdBrowser/ladybird.git synced 2025-06-08 05:27:14 +09:00
Code Activity

LibWebView: Do not use AK::format to format search engine URLs

Browse source 
This is to prepare for custom search engines. If we use AK::format, it
would be trivial for a user (or bad actor) to come up with a template
search engine URL that ultimately crashes the browser due to internal
assertions in AK::format. For example:

    https://example.com/crash={1}

Rather than coming up with a complicated pre-format validator, let's
just not use AK::format. Custom URLs will signify their template query
parameters with "%s". So we can do the same with our built-in engines.
When it comes time to format the URL, we will do a simple string
replacement.
This commit is contained in:
Timothy Flynn 2025-04-04 17:32:04 -04:00 • committed by Andreas Kling
parent cbee476dac
commit dbf4b189a4
Notes: github-actions[bot] 2025-04-06 11:46:09 +00:00
9 changed files with 44 additions and 56 deletions
Download patch file Download diff file Expand all files Collapse all files

46
Libraries/LibWebView/SearchEngine.cpp
View file

@ -5,21 +5,22 @@
*/ */
#include <AK/Find.h> #include <AK/Find.h>
#include <LibURL/URL.h>
#include <LibWebView/SearchEngine.h> #include <LibWebView/SearchEngine.h>
namespace WebView { namespace WebView {
static auto builtin_search_engines = to_array<SearchEngine>({ static auto builtin_search_engines = to_array<SearchEngine>({
{ "Bing"_string, "https://www.bing.com/search?q={}"_string }, { "Bing"_string, "https://www.bing.com/search?q=%s"_string },
{ "Brave"_string, "https://search.brave.com/search?q={}"_string }, { "Brave"_string, "https://search.brave.com/search?q=%s"_string },
{ "DuckDuckGo"_string, "https://duckduckgo.com/?q={}"_string }, { "DuckDuckGo"_string, "https://duckduckgo.com/?q=%s"_string },
{ "Ecosia"_string, "https://ecosia.org/search?q={}"_string }, { "Ecosia"_string, "https://ecosia.org/search?q=%s"_string },
{ "Google"_string, "https://www.google.com/search?q={}"_string }, { "Google"_string, "https://www.google.com/search?q=%s"_string },
{ "Kagi"_string, "https://kagi.com/search?q={}"_string }, { "Kagi"_string, "https://kagi.com/search?q=%s"_string },
{ "Mojeek"_string, "https://www.mojeek.com/search?q={}"_string }, { "Mojeek"_string, "https://www.mojeek.com/search?q=%s"_string },
{ "Startpage"_string, "https://startpage.com/search?q={}"_string }, { "Startpage"_string, "https://startpage.com/search?q=%s"_string },
{ "Yahoo"_string, "https://search.yahoo.com/search?p={}"_string }, { "Yahoo"_string, "https://search.yahoo.com/search?p=%s"_string },
{ "Yandex"_string, "https://yandex.com/search/?text={}"_string }, { "Yandex"_string, "https://yandex.com/search/?text=%s"_string },
}); });
ReadonlySpan<SearchEngine> search_engines() ReadonlySpan<SearchEngine> search_engines()
@ -34,29 +35,20 @@ Optional<SearchEngine> find_search_engine_by_name(StringView name)
}); });
} }
Optional<SearchEngine const&> find_search_engine_by_query_url(StringView query_url) String SearchEngine::format_search_query_for_display(StringView query) const
{
return find_value(builtin_search_engines, [&](auto const& engine) {
return engine.query_url == query_url;
});
}
String format_search_query_for_display(StringView query_url, StringView query)
{ {
static constexpr auto MAX_SEARCH_STRING_LENGTH = 32; static constexpr auto MAX_SEARCH_STRING_LENGTH = 32;
if (auto search_engine = find_search_engine_by_query_url(query_url); search_engine.has_value()) { return MUST(String::formatted("Search {} for \"{:.{}}{}\"",
return MUST(String::formatted("Search {} for \"{:.{}}{}\"", name,
search_engine->name,
query,
MAX_SEARCH_STRING_LENGTH,
query.length() > MAX_SEARCH_STRING_LENGTH ? "..."sv : ""sv));
}
return MUST(String::formatted("Search for \"{:.{}}{}\"",
query, query,
MAX_SEARCH_STRING_LENGTH, MAX_SEARCH_STRING_LENGTH,
query.length() > MAX_SEARCH_STRING_LENGTH ? "..."sv : ""sv)); query.length() > MAX_SEARCH_STRING_LENGTH ? "..."sv : ""sv));
} }
String SearchEngine::format_search_query_for_navigation(StringView query) const
{
return MUST(query_url.replace("%s"sv, URL::percent_encode(query), ReplaceMode::All));
}
} }

5
Libraries/LibWebView/SearchEngine.h
View file

@ -13,13 +13,14 @@
namespace WebView { namespace WebView {
struct SearchEngine { struct SearchEngine {
String format_search_query_for_display(StringView query) const;
String format_search_query_for_navigation(StringView query) const;
String name; String name;
String query_url; String query_url;
}; };
ReadonlySpan<SearchEngine> search_engines(); ReadonlySpan<SearchEngine> search_engines();
Optional<SearchEngine> find_search_engine_by_name(StringView name); Optional<SearchEngine> find_search_engine_by_name(StringView name);
Optional<SearchEngine const&> find_search_engine_by_query_url(StringView query_url);
String format_search_query_for_display(StringView query_url, StringView query);
} }

6
Libraries/LibWebView/URL.cpp
View file

@ -1,5 +1,5 @@
/* /*
* Copyright (c) 2023-2025, Tim Flynn <trflynn89@serenityos.org> * Copyright (c) 2023-2025, Tim Flynn <trflynn89@ladybird.org>
* Copyright (c) 2023, Cameron Youell <cameronyouell@gmail.com> * Copyright (c) 2023, Cameron Youell <cameronyouell@gmail.com>
* Copyright (c) 2025, Manuel Zahariev <manuel@duck.com> * Copyright (c) 2025, Manuel Zahariev <manuel@duck.com>
* *
@ -13,13 +13,13 @@
namespace WebView { namespace WebView {
Optional<URL::URL> sanitize_url(StringView location, Optional<StringView> search_engine, AppendTLD append_tld) Optional<URL::URL> sanitize_url(StringView location, Optional<SearchEngine> const& search_engine, AppendTLD append_tld)
{ {
auto search_url_or_error = [&]() -> Optional<URL::URL> { auto search_url_or_error = [&]() -> Optional<URL::URL> {
if (!search_engine.has_value()) if (!search_engine.has_value())
return {}; return {};
return URL::Parser::basic_parse(MUST(String::formatted(*search_engine, URL::percent_encode(location)))); return URL::Parser::basic_parse(search_engine->format_search_query_for_navigation(location));
}; };
location = location.trim_whitespace(); location = location.trim_whitespace();

5
Libraries/LibWebView/URL.h
View file

@ -1,5 +1,5 @@
/* /*
* Copyright (c) 2023, Tim Flynn <trflynn89@serenityos.org> * Copyright (c) 2023-2025, Tim Flynn <trflynn89@ladybird.org>
* *
* SPDX-License-Identifier: BSD-2-Clause * SPDX-License-Identifier: BSD-2-Clause
*/ */
@ -9,6 +9,7 @@
#include <AK/Optional.h> #include <AK/Optional.h>
#include <AK/StringView.h> #include <AK/StringView.h>
#include <LibURL/URL.h> #include <LibURL/URL.h>
#include <LibWebView/SearchEngine.h>
namespace WebView { namespace WebView {
@ -16,7 +17,7 @@ enum class AppendTLD {
No, No,
Yes, Yes,
}; };
Optional<URL::URL> sanitize_url(StringView, Optional<StringView> search_engine = {}, AppendTLD = AppendTLD::No); Optional<URL::URL> sanitize_url(StringView, Optional<SearchEngine> const& search_engine = {}, AppendTLD = AppendTLD::No);
Vector<URL::URL> sanitize_urls(ReadonlySpan<ByteString> raw_urls, URL::URL const& new_tab_page_url); Vector<URL::URL> sanitize_urls(ReadonlySpan<ByteString> raw_urls, URL::URL const& new_tab_page_url);
struct URLParts { struct URLParts {

18
Tests/LibWebView/TestWebViewURL.cpp
View file

@ -6,8 +6,14 @@
*/ */
#include <LibTest/TestCase.h> #include <LibTest/TestCase.h>
#include <LibWebView/SearchEngine.h>
#include <LibWebView/URL.h> #include <LibWebView/URL.h>
static WebView::SearchEngine s_test_engine {
.name = "Test"_string,
.query_url = "https://ecosia.org/search?q=%s"_string
};
static void compare_url_parts(StringView url, WebView::URLParts const& expected) static void compare_url_parts(StringView url, WebView::URLParts const& expected)
{ {
auto result = WebView::break_url_into_parts(url); auto result = WebView::break_url_into_parts(url);
@ -28,9 +34,7 @@ static bool is_sanitized_url_the_same(StringView url)
static void expect_url_equals_sanitized_url(StringView test_url, StringView url, WebView::AppendTLD append_tld = WebView::AppendTLD::No) static void expect_url_equals_sanitized_url(StringView test_url, StringView url, WebView::AppendTLD append_tld = WebView::AppendTLD::No)
{ {
StringView const search_engine_url = "https://ecosia.org/search?q={}"sv; auto sanitized_url = WebView::sanitize_url(url, s_test_engine, append_tld);
auto sanitized_url = WebView::sanitize_url(url, search_engine_url, append_tld);
EXPECT(sanitized_url.has_value()); EXPECT(sanitized_url.has_value());
EXPECT_EQ(sanitized_url->to_string(), test_url); EXPECT_EQ(sanitized_url->to_string(), test_url);
@ -38,13 +42,11 @@ static void expect_url_equals_sanitized_url(StringView test_url, StringView url,
static void expect_search_url_equals_sanitized_url(StringView url) static void expect_search_url_equals_sanitized_url(StringView url)
{ {
StringView const search_engine_url = "https://ecosia.org/search?q={}"sv; auto search_url = s_test_engine.format_search_query_for_navigation(url);
auto const search_url = String::formatted(search_engine_url, URL::percent_encode(url)); auto sanitized_url = WebView::sanitize_url(url, s_test_engine);
auto sanitized_url = WebView::sanitize_url(url, search_engine_url);
EXPECT(sanitized_url.has_value()); EXPECT(sanitized_url.has_value());
EXPECT_EQ(sanitized_url->to_string(), search_url.value()); EXPECT_EQ(sanitized_url->to_string(), search_url);
} }
TEST_CASE(invalid_url) TEST_CASE(invalid_url)

4
UI/AppKit/Interface/LadybirdWebView.mm
View file

@ -631,7 +631,7 @@ static void copy_data_to_clipboard(StringView data, NSPasteboardType pasteboard_
TemporaryChange change_url { m_context_menu_search_text, move(selected_text) }; TemporaryChange change_url { m_context_menu_search_text, move(selected_text) };
if (m_context_menu_search_text.has_value()) { if (m_context_menu_search_text.has_value()) {
auto action_text = WebView::format_search_query_for_display(search_engine->query_url, *m_context_menu_search_text); auto action_text = search_engine->format_search_query_for_display(*m_context_menu_search_text);
[search_selected_text_menu_item setTitle:Ladybird::string_to_ns_string(action_text)]; [search_selected_text_menu_item setTitle:Ladybird::string_to_ns_string(action_text)];
[search_selected_text_menu_item setHidden:NO]; [search_selected_text_menu_item setHidden:NO];
} else { } else {
@ -1135,7 +1135,7 @@ static void copy_data_to_clipboard(StringView data, NSPasteboardType pasteboard_
if (!search_engine.has_value()) if (!search_engine.has_value())
return; return;
auto url_string = MUST(String::formatted(search_engine->query_url, URL::percent_encode(*m_context_menu_search_text))); auto url_string = search_engine->format_search_query_for_navigation(*m_context_menu_search_text);
auto url = URL::Parser::basic_parse(url_string); auto url = URL::Parser::basic_parse(url_string);
VERIFY(url.has_value()); VERIFY(url.has_value());
[self.observer onCreateNewTab:url.release_value() activateTab:Web::HTML::ActivateTab::Yes]; [self.observer onCreateNewTab:url.release_value() activateTab:Web::HTML::ActivateTab::Yes];

6
UI/AppKit/Interface/TabController.mm
View file

@ -301,11 +301,7 @@ static NSString* const TOOLBAR_TAB_OVERVIEW_IDENTIFIER = @"ToolbarTabOverviewIde
- (BOOL)navigateToLocation:(String)location - (BOOL)navigateToLocation:(String)location
{ {
Optional<StringView> search_engine_url; if (auto url = WebView::sanitize_url(location, WebView::Application::settings().search_engine()); url.has_value()) {
if (auto const& search_engine = WebView::Application::settings().search_engine(); search_engine.has_value())
search_engine_url = search_engine->query_url;
if (auto url = WebView::sanitize_url(location, search_engine_url); url.has_value()) {
[self loadURL:*url]; [self loadURL:*url];
} }

6
UI/Qt/LocationEdit.cpp
View file

@ -36,16 +36,12 @@ LocationEdit::LocationEdit(QWidget* parent)
clearFocus(); clearFocus();
Optional<StringView> search_engine_url;
if (auto const& search_engine = WebView::Application::settings().search_engine(); search_engine.has_value())
search_engine_url = search_engine->query_url;
auto query = ak_string_from_qstring(text()); auto query = ak_string_from_qstring(text());
auto ctrl_held = QApplication::keyboardModifiers() & Qt::ControlModifier; auto ctrl_held = QApplication::keyboardModifiers() & Qt::ControlModifier;
auto append_tld = ctrl_held ? WebView::AppendTLD::Yes : WebView::AppendTLD::No; auto append_tld = ctrl_held ? WebView::AppendTLD::Yes : WebView::AppendTLD::No;
if (auto url = WebView::sanitize_url(query, search_engine_url, append_tld); url.has_value()) if (auto url = WebView::sanitize_url(query, WebView::Application::settings().search_engine(), append_tld); url.has_value())
set_url(url.release_value()); set_url(url.release_value());
}); });

4
UI/Qt/Tab.cpp
View file

@ -459,7 +459,7 @@ Tab::Tab(BrowserWindow* window, RefPtr<WebView::WebContentClient> parent_client,
if (!search_engine.has_value()) if (!search_engine.has_value())
return; return;
auto url_string = MUST(String::formatted(search_engine->query_url, URL::percent_encode(*m_page_context_menu_search_text))); auto url_string = search_engine->format_search_query_for_navigation(*m_page_context_menu_search_text);
auto url = URL::Parser::basic_parse(url_string); auto url = URL::Parser::basic_parse(url_string);
VERIFY(url.has_value()); VERIFY(url.has_value());
@ -531,7 +531,7 @@ Tab::Tab(BrowserWindow* window, RefPtr<WebView::WebContentClient> parent_client,
TemporaryChange change_url { m_page_context_menu_search_text, AK::move(selected_text) }; TemporaryChange change_url { m_page_context_menu_search_text, AK::move(selected_text) };
if (m_page_context_menu_search_text.has_value()) { if (m_page_context_menu_search_text.has_value()) {
auto action_text = WebView::format_search_query_for_display(search_engine->query_url, *m_page_context_menu_search_text); auto action_text = search_engine->format_search_query_for_display(*m_page_context_menu_search_text);
search_selected_text_action->setText(qstring_from_ak_string(action_text)); search_selected_text_action->setText(qstring_from_ak_string(action_text));
search_selected_text_action->setVisible(true); search_selected_text_action->setVisible(true);
} else { } else {