Browser: Sanitize user-provided URLs with LibWebView

Author: https://github.com/trflynn89
Commit: https://github.com/SerenityOS/serenity/commit/d2c7e1ea7d
Pull-request: https://github.com/SerenityOS/serenity/pull/21435
Reviewed-by: https://github.com/ADKaster ✅

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<URL> 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<Browser::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() });
 }

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<URL> const&);
 
     void build_menus();
     ErrorOr<void> load_search_engines(GUI::Menu& settings_menu);

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)

Userland/Applications/Browser/Tab.cpp

@@ -23,7 +23,6 @@
 #include <Applications/Browser/TabGML.h>
 #include <Applications/BrowserSettings/Defaults.h>
 #include <LibConfig/Client.h>
-#include <LibFileSystem/FileSystem.h>
 #include <LibGUI/Action.h>
 #include <LibGUI/Application.h>
 #include <LibGUI/BoxLayout.h>
@@ -39,13 +38,13 @@
 #include <LibGUI/Toolbar.h>
 #include <LibGUI/ToolbarContainer.h>
 #include <LibGUI/Window.h>
-#include <LibPublicSuffix/URL.h>
 #include <LibWeb/HTML/BrowsingContext.h>
 #include <LibWeb/HTML/SyntaxHighlighter/SyntaxHighlighter.h>
 #include <LibWeb/Layout/BlockContainer.h>
 #include <LibWeb/Layout/Viewport.h>
 #include <LibWeb/Loader/ResourceLoader.h>
 #include <LibWebView/OutOfProcessWebView.h>
+#include <LibWebView/URL.h>
 
 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<URL> 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);

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<String> text_override = {}, i32 count_waiting = 0);
     void close_sub_widgets();
 
-    enum class MayAppendTLD {
-        No,
-        Yes
-    };
-
-    Optional<URL> url_from_location_bar(MayAppendTLD = MayAppendTLD::No);
-
     WebView::History m_history;
 
     RefPtr<WebView::OutOfProcessWebView> m_web_content_view;
@@ -167,6 +161,4 @@ private:
     bool m_is_history_navigation { false };
 };
 
-URL url_from_user_input(DeprecatedString const& input);
-
 }

Userland/Applications/Browser/main.cpp

@@ -17,7 +17,6 @@
 #include <LibCore/StandardPaths.h>
 #include <LibCore/System.h>
 #include <LibDesktop/Launcher.h>
-#include <LibFileSystem/FileSystem.h>
 #include <LibGUI/Application.h>
 #include <LibGUI/BoxLayout.h>
 #include <LibGUI/Icon.h>
@@ -28,6 +27,7 @@
 #include <LibWebView/Database.h>
 #include <LibWebView/OutOfProcessWebView.h>
 #include <LibWebView/RequestServerAdapter.h>
+#include <LibWebView/URL.h>
 #include <unistd.h>
 
 namespace Browser {
@@ -95,7 +95,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
 
     TRY(Core::System::pledge("stdio recvfd sendfd unix fattr cpath rpath wpath proc exec"));
 
-    Vector<DeprecatedString> specified_urls;
+    Vector<StringView> 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<int> serenity_main(Main::Arguments arguments)
         }
     }
 
-    auto url_from_argument_string = [](DeprecatedString const& string) -> ErrorOr<URL> {
-        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<URL> 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<int> 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());