From d2c7e1ea7db060c1f01a740fab65f3e35a45545b Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Fri, 13 Oct 2023 10:31:52 -0400 Subject: [PATCH] Browser: Sanitize user-provided URLs with LibWebView --- .../Applications/Browser/BrowserWindow.cpp | 13 ++-- Userland/Applications/Browser/BrowserWindow.h | 6 +- Userland/Applications/Browser/CMakeLists.txt | 2 +- Userland/Applications/Browser/Tab.cpp | 65 ++----------------- Userland/Applications/Browser/Tab.h | 12 +--- Userland/Applications/Browser/main.cpp | 26 ++++---- 6 files changed, 31 insertions(+), 93 deletions(-) diff --git a/Userland/Applications/Browser/BrowserWindow.cpp b/Userland/Applications/Browser/BrowserWindow.cpp index 887a424404..a9d0fa7520 100644 --- a/Userland/Applications/Browser/BrowserWindow.cpp +++ b/Userland/Applications/Browser/BrowserWindow.cpp @@ -57,7 +57,7 @@ static DeprecatedString search_engines_file_path() return builder.to_deprecated_string(); } -BrowserWindow::BrowserWindow(WebView::CookieJar& cookie_jar, URL url) +BrowserWindow::BrowserWindow(WebView::CookieJar& cookie_jar, Vector const& initial_urls) : m_cookie_jar(cookie_jar) , m_window_actions(*this) { @@ -102,7 +102,7 @@ BrowserWindow::BrowserWindow(WebView::CookieJar& cookie_jar, URL url) }; m_window_actions.on_create_new_tab = [this] { - create_new_tab(Browser::url_from_user_input(Browser::g_new_tab_url), Web::HTML::ActivateTab::Yes); + create_new_tab(Browser::g_new_tab_url, Web::HTML::ActivateTab::Yes); }; m_window_actions.on_create_new_window = [this] { @@ -148,7 +148,8 @@ BrowserWindow::BrowserWindow(WebView::CookieJar& cookie_jar, URL url) build_menus(); - create_new_tab(move(url), Web::HTML::ActivateTab::Yes); + for (size_t i = 0; i < initial_urls.size(); ++i) + create_new_tab(initial_urls[i], (i == 0) ? Web::HTML::ActivateTab::Yes : Web::HTML::ActivateTab::No); } void BrowserWindow::build_menus() @@ -214,7 +215,7 @@ void BrowserWindow::build_menus() m_go_back_action = GUI::CommonActions::make_go_back_action([this](auto&) { active_tab().go_back(); }, this); m_go_forward_action = GUI::CommonActions::make_go_forward_action([this](auto&) { active_tab().go_forward(); }, this); - m_go_home_action = GUI::CommonActions::make_go_home_action([this](auto&) { active_tab().load(Browser::url_from_user_input(g_home_url)); }, this); + m_go_home_action = GUI::CommonActions::make_go_home_action([this](auto&) { active_tab().load(g_home_url); }, this); m_go_home_action->set_status_tip("Go to home page"_string); m_reload_action = GUI::CommonActions::make_reload_action([this](auto&) { active_tab().reload(); }, this); m_reload_action->set_status_tip("Reload current page"_string); @@ -558,7 +559,7 @@ void BrowserWindow::set_window_title_for_tab(Tab const& tab) set_title(DeprecatedString::formatted("{} - Ladybird", title.is_empty() ? url.to_deprecated_string() : title)); } -Tab& BrowserWindow::create_new_tab(URL url, Web::HTML::ActivateTab activate) +Tab& BrowserWindow::create_new_tab(URL const& url, Web::HTML::ActivateTab activate) { auto& new_tab = m_tab_widget->add_tab("New tab"_string, *this); @@ -649,7 +650,7 @@ Tab& BrowserWindow::create_new_tab(URL url, Web::HTML::ActivateTab activate) return new_tab; } -void BrowserWindow::create_new_window(URL url) +void BrowserWindow::create_new_window(URL const& url) { GUI::Process::spawn_or_show_error(this, "/bin/Browser"sv, Array { url.to_deprecated_string() }); } diff --git a/Userland/Applications/Browser/BrowserWindow.h b/Userland/Applications/Browser/BrowserWindow.h index dbf4e856f1..10aa9ef62c 100644 --- a/Userland/Applications/Browser/BrowserWindow.h +++ b/Userland/Applications/Browser/BrowserWindow.h @@ -29,8 +29,8 @@ public: GUI::TabWidget& tab_widget(); Tab& active_tab(); - Tab& create_new_tab(URL, Web::HTML::ActivateTab activate); - void create_new_window(URL); + Tab& create_new_tab(URL const&, Web::HTML::ActivateTab activate); + void create_new_window(URL const&); GUI::Action& go_back_action() { return *m_go_back_action; } GUI::Action& go_forward_action() { return *m_go_forward_action; } @@ -51,7 +51,7 @@ public: void broadcast_window_size(Gfx::IntSize); private: - BrowserWindow(WebView::CookieJar&, URL); + BrowserWindow(WebView::CookieJar&, Vector const&); void build_menus(); ErrorOr load_search_engines(GUI::Menu& settings_menu); diff --git a/Userland/Applications/Browser/CMakeLists.txt b/Userland/Applications/Browser/CMakeLists.txt index 1ca154e2d1..0cc7c4eb5f 100644 --- a/Userland/Applications/Browser/CMakeLists.txt +++ b/Userland/Applications/Browser/CMakeLists.txt @@ -38,5 +38,5 @@ set(GENERATED_SOURCES ) serenity_app(Browser ICON app-browser) -target_link_libraries(Browser PRIVATE LibCore LibFileSystem LibWebView LibWeb LibProtocol LibPublicSuffix LibGUI LibDesktop LibConfig LibGfx LibIPC LibJS LibLocale LibMain LibSyntax) +target_link_libraries(Browser PRIVATE LibCore LibWebView LibWeb LibProtocol LibGUI LibDesktop LibConfig LibGfx LibIPC LibJS LibLocale LibMain LibSyntax) link_with_locale_data(Browser) diff --git a/Userland/Applications/Browser/Tab.cpp b/Userland/Applications/Browser/Tab.cpp index c180d368e0..7822198bcd 100644 --- a/Userland/Applications/Browser/Tab.cpp +++ b/Userland/Applications/Browser/Tab.cpp @@ -23,7 +23,6 @@ #include #include #include -#include #include #include #include @@ -39,13 +38,13 @@ #include #include #include -#include #include #include #include #include #include #include +#include namespace Browser { @@ -54,19 +53,6 @@ Tab::~Tab() close_sub_widgets(); } -URL url_from_user_input(DeprecatedString const& input) -{ - URL url_with_http_schema = URL(DeprecatedString::formatted("https://{}", input)); - if (url_with_http_schema.is_valid() && url_with_http_schema.port().has_value()) - return url_with_http_schema; - - URL url = URL(input); - if (url.is_valid()) - return url; - - return url_with_http_schema; -} - void Tab::start_download(const URL& url) { auto window = GUI::Window::construct(&this->window()); @@ -166,7 +152,7 @@ Tab::Tab(BrowserWindow& window) auto& go_home_button = toolbar.add_action(window.go_home_action()); go_home_button.set_allowed_mouse_buttons_for_pressing(GUI::MouseButton::Primary | GUI::MouseButton::Middle); go_home_button.on_middle_mouse_click = [&](auto) { - on_tab_open_request(Browser::url_from_user_input(g_home_url)); + on_tab_open_request(g_home_url); }; toolbar.add_action(window.reload_action()); @@ -175,15 +161,13 @@ Tab::Tab(BrowserWindow& window) m_location_box->set_placeholder("Search or enter address"sv); m_location_box->on_return_pressed = [this] { - auto url = url_from_location_bar(); - if (url.has_value()) - load(url.release_value()); + if (auto url = WebView::sanitize_url(m_location_box->text(), g_search_engine, WebView::AppendTLD::No); url.has_value()) + load(*url); }; m_location_box->on_ctrl_return_pressed = [this] { - auto url = url_from_location_bar(MayAppendTLD::Yes); - if (url.has_value()) - load(url.release_value()); + if (auto url = WebView::sanitize_url(m_location_box->text(), g_search_engine, WebView::AppendTLD::Yes); url.has_value()) + load(*url); }; m_location_box->add_custom_context_menu_action(GUI::Action::create("Paste && Go", [this](auto&) { @@ -700,42 +684,7 @@ void Tab::update_reset_zoom_button() } } -Optional Tab::url_from_location_bar(MayAppendTLD may_append_tld) -{ - DeprecatedString text = m_location_box->text(); - if (text.starts_with('/') && FileSystem::is_regular_file(text)) { - auto real_path = FileSystem::real_path(text); - if (real_path.is_error()) { - return {}; - } - - return URL::create_with_file_scheme(real_path.value().to_deprecated_string()); - } - - StringBuilder builder; - builder.append(text); - if (may_append_tld == MayAppendTLD::Yes) { - // FIXME: Expand the list of top level domains. - if (!(text.ends_with(".com"sv) || text.ends_with(".net"sv) || text.ends_with(".org"sv))) { - builder.append(".com"sv); - } - } - auto final_text = builder.to_deprecated_string(); - - auto error_or_absolute_url = PublicSuffix::absolute_url(final_text); - if (error_or_absolute_url.is_error()) { - if (g_search_engine.is_empty()) { - GUI::MessageBox::show(&this->window(), "Select a search engine in the Settings menu before searching."sv, "No search engine selected"sv, GUI::MessageBox::Type::Information); - return {}; - } - - return URL(g_search_engine.replace("{}"sv, URL::percent_encode(final_text), ReplaceMode::FirstOnly)); - } - - return URL(error_or_absolute_url.release_value()); -} - -void Tab::load(const URL& url, LoadType load_type) +void Tab::load(URL const& url, LoadType load_type) { m_is_history_navigation = (load_type == LoadType::HistoryNavigation); m_web_content_view->load(url); diff --git a/Userland/Applications/Browser/Tab.h b/Userland/Applications/Browser/Tab.h index 31d90706a5..bdb679367c 100644 --- a/Userland/Applications/Browser/Tab.h +++ b/Userland/Applications/Browser/Tab.h @@ -45,7 +45,8 @@ public: HistoryNavigation, }; - void load(const URL&, LoadType = LoadType::Normal); + void load(URL const&, LoadType = LoadType::Normal); + void reload(); void go_back(int steps = 1); void go_forward(int steps = 1); @@ -115,13 +116,6 @@ private: void update_status(Optional text_override = {}, i32 count_waiting = 0); void close_sub_widgets(); - enum class MayAppendTLD { - No, - Yes - }; - - Optional url_from_location_bar(MayAppendTLD = MayAppendTLD::No); - WebView::History m_history; RefPtr m_web_content_view; @@ -167,6 +161,4 @@ private: bool m_is_history_navigation { false }; }; -URL url_from_user_input(DeprecatedString const& input); - } diff --git a/Userland/Applications/Browser/main.cpp b/Userland/Applications/Browser/main.cpp index fa8c2565bb..b164d1e069 100644 --- a/Userland/Applications/Browser/main.cpp +++ b/Userland/Applications/Browser/main.cpp @@ -17,7 +17,6 @@ #include #include #include -#include #include #include #include @@ -28,6 +27,7 @@ #include #include #include +#include #include namespace Browser { @@ -95,7 +95,7 @@ ErrorOr serenity_main(Main::Arguments arguments) TRY(Core::System::pledge("stdio recvfd sendfd unix fattr cpath rpath wpath proc exec")); - Vector specified_urls; + Vector specified_urls; Core::ArgsParser args_parser; args_parser.add_positional_argument(specified_urls, "URLs to open", "url", Core::ArgsParser::Required::No); @@ -158,19 +158,18 @@ ErrorOr serenity_main(Main::Arguments arguments) } } - auto url_from_argument_string = [](DeprecatedString const& string) -> ErrorOr { - if (FileSystem::exists(string)) { - return URL::create_with_file_scheme(TRY(FileSystem::real_path(string)).to_deprecated_string()); - } - return Browser::url_from_user_input(string); - }; + Vector initial_urls; - URL first_url = Browser::url_from_user_input(Browser::g_home_url); - if (!specified_urls.is_empty()) - first_url = TRY(url_from_argument_string(specified_urls.first())); + for (auto specified_url : specified_urls) { + if (auto url = WebView::sanitize_url(specified_url); url.has_value()) + initial_urls.append(url.release_value()); + } + + if (initial_urls.is_empty()) + initial_urls.append(Browser::g_home_url); auto cookie_jar = TRY(WebView::CookieJar::create(*database)); - auto window = Browser::BrowserWindow::construct(cookie_jar, first_url); + auto window = Browser::BrowserWindow::construct(cookie_jar, initial_urls); auto content_filters_watcher = TRY(Core::FileWatcher::create()); content_filters_watcher->on_change = [&](Core::FileWatcherEvent const&) { @@ -213,9 +212,6 @@ ErrorOr serenity_main(Main::Arguments arguments) } }; - for (size_t i = 1; i < specified_urls.size(); ++i) - window->create_new_tab(TRY(url_from_argument_string(specified_urls[i])), Web::HTML::ActivateTab::No); - window->show(); window->broadcast_window_position(window->position());