Browser+LibWeb+WebContent: Parse cookies in the OOP tab

To protect the main Browser process against nefarious cookies, parse the
cookies out-of-process and then send the parsed result over IPC to the
main process. This way, if the cookie parser blows up, only that tab
will be affected.

19 files changed, 79 insertions, 26 deletions

diff --git a/Userland/Applications/Browser/CookieJar.cpp b/Userland/Applications/Browser/CookieJar.cpp
index 6b8e07f488..6122f9774f 100644
--- a/Userland/Applications/Browser/CookieJar.cpp
+++ b/Userland/Applications/Browser/CookieJar.cpp
@@ -56,17 +56,13 @@ String CookieJar::get_cookie(const URL& url, Web::Cookie::Source source)
     return builder.build();
 }
 
-void CookieJar::set_cookie(const URL& url, const String& cookie_string, Web::Cookie::Source source)
+void CookieJar::set_cookie(const URL& url, const Web::Cookie::ParsedCookie& parsed_cookie, Web::Cookie::Source source)
 {
     auto domain = canonicalize_domain(url);
     if (!domain.has_value())
         return;
 
-    auto parsed_cookie = Web::Cookie::parse_cookie(cookie_string);
-    if (!parsed_cookie.has_value())
-        return;
-
-    store_cookie(parsed_cookie.value(), url, move(domain.value()), source);
+    store_cookie(parsed_cookie, url, move(domain.value()), source);
     purge_expired_cookies();
 }
 
@@ -176,12 +172,12 @@ String CookieJar::default_path(const URL& url)
     return uri_path.substring(0, last_separator);
 }
 
-void CookieJar::store_cookie(Web::Cookie::ParsedCookie& parsed_cookie, const URL& url, String canonicalized_domain, Web::Cookie::Source source)
+void CookieJar::store_cookie(const Web::Cookie::ParsedCookie& parsed_cookie, const URL& url, String canonicalized_domain, Web::Cookie::Source source)
 {
     // https://tools.ietf.org/html/rfc6265#section-5.3
 
     // 2. Create a new cookie with name cookie-name, value cookie-value. Set the creation-time and the last-access-time to the current date and time.
-    Web::Cookie::Cookie cookie { move(parsed_cookie.name), move(parsed_cookie.value) };
+    Web::Cookie::Cookie cookie { parsed_cookie.name, parsed_cookie.value };
     cookie.creation_time = Core::DateTime::now();
     cookie.last_access_time = cookie.creation_time;
 
@@ -189,12 +185,12 @@ void CookieJar::store_cookie(Web::Cookie::ParsedCookie& parsed_cookie, const URL
         // 3. If the cookie-attribute-list contains an attribute with an attribute-name of "Max-Age": Set the cookie's persistent-flag to true.
         // Set the cookie's expiry-time to attribute-value of the last attribute in the cookie-attribute-list with an attribute-name of "Max-Age".
         cookie.persistent = true;
-        cookie.expiry_time = move(parsed_cookie.expiry_time_from_max_age_attribute.value());
+        cookie.expiry_time = parsed_cookie.expiry_time_from_max_age_attribute.value();
     } else if (parsed_cookie.expiry_time_from_expires_attribute.has_value()) {
         // If the cookie-attribute-list contains an attribute with an attribute-name of "Expires": Set the cookie's persistent-flag to true.
         // Set the cookie's expiry-time to attribute-value of the last attribute in the cookie-attribute-list with an attribute-name of "Expires".
         cookie.persistent = true;
-        cookie.expiry_time = move(parsed_cookie.expiry_time_from_expires_attribute.value());
+        cookie.expiry_time = parsed_cookie.expiry_time_from_expires_attribute.value();
     } else {
         // Set the cookie's persistent-flag to false. Set the cookie's expiry-time to the latest representable gddate.
         cookie.persistent = false;
@@ -204,7 +200,7 @@ void CookieJar::store_cookie(Web::Cookie::ParsedCookie& parsed_cookie, const URL
     // 4. If the cookie-attribute-list contains an attribute with an attribute-name of "Domain":
     if (parsed_cookie.domain.has_value()) {
         // Let the domain-attribute be the attribute-value of the last attribute in the cookie-attribute-list with an attribute-name of "Domain".
-        cookie.domain = move(parsed_cookie.domain.value());
+        cookie.domain = parsed_cookie.domain.value();
     }
 
     // 5. If the user agent is configured to reject "public suffixes" and the domain-attribute is a public suffix:
@@ -227,7 +223,7 @@ void CookieJar::store_cookie(Web::Cookie::ParsedCookie& parsed_cookie, const URL
     // 7. If the cookie-attribute-list contains an attribute with an attribute-name of "Path":
     if (parsed_cookie.path.has_value()) {
         // Set the cookie's path to attribute-value of the last attribute in the cookie-attribute-list with an attribute-name of "Path".
-        cookie.path = move(parsed_cookie.path.value());
+        cookie.path = parsed_cookie.path.value();
     } else {
         cookie.path = default_path(url);
     }
diff --git a/Userland/Applications/Browser/CookieJar.h b/Userland/Applications/Browser/CookieJar.h
index b3f69da1ca..c1ac54942e 100644
--- a/Userland/Applications/Browser/CookieJar.h
+++ b/Userland/Applications/Browser/CookieJar.h
@@ -47,7 +47,7 @@ struct CookieStorageKey {
 class CookieJar {
 public:
     String get_cookie(const URL& url, Web::Cookie::Source source);
-    void set_cookie(const URL& url, const String& cookie, Web::Cookie::Source source);
+    void set_cookie(const URL& url, const Web::Cookie::ParsedCookie& parsed_cookie, Web::Cookie::Source source);
     void dump_cookies() const;
 
 private:
@@ -56,7 +56,7 @@ private:
     static bool path_matches(const String& request_path, const String& cookie_path);
     static String default_path(const URL& url);
 
-    void store_cookie(Web::Cookie::ParsedCookie& parsed_cookie, const URL& url, String canonicalized_domain, Web::Cookie::Source source);
+    void store_cookie(const Web::Cookie::ParsedCookie& parsed_cookie, const URL& url, String canonicalized_domain, Web::Cookie::Source source);
     Vector<Web::Cookie::Cookie*> get_matching_cookies(const URL& url, const String& canonicalized_domain, Web::Cookie::Source source);
     void purge_expired_cookies();
 
diff --git a/Userland/Applications/Browser/Tab.h b/Userland/Applications/Browser/Tab.h
index 9311b0ca64..9e0add02be 100644
--- a/Userland/Applications/Browser/Tab.h
+++ b/Userland/Applications/Browser/Tab.h
@@ -72,7 +72,7 @@ public:
     Function<void(Tab&)> on_tab_close_request;
     Function<void(const Gfx::Bitmap&)> on_favicon_change;
     Function<String(const URL& url, Web::Cookie::Source source)> on_get_cookie;
-    Function<void(const URL& url, const String& cookie, Web::Cookie::Source source)> on_set_cookie;
+    Function<void(const URL& url, const Web::Cookie::ParsedCookie& cookie, Web::Cookie::Source source)> on_set_cookie;
     Function<void()> on_dump_cookies;
 
     const String& title() const { return m_title; }
diff --git a/Userland/Libraries/LibWeb/Cookie/ParsedCookie.cpp b/Userland/Libraries/LibWeb/Cookie/ParsedCookie.cpp
index 8f4b7e058e..6d0d0d4940 100644
--- a/Userland/Libraries/LibWeb/Cookie/ParsedCookie.cpp
+++ b/Userland/Libraries/LibWeb/Cookie/ParsedCookie.cpp
@@ -25,7 +25,10 @@
  */
 
 #include "ParsedCookie.h"
+#include <AK/StdLibExtras.h>
 #include <AK/Vector.h>
+#include <LibIPC/Decoder.h>
+#include <LibIPC/Encoder.h>
 #include <ctype.h>
 
 namespace Web::Cookie {
@@ -351,3 +354,39 @@ Optional<Core::DateTime> parse_date_time(StringView date_string)
 }
 
 }
+
+bool IPC::encode(IPC::Encoder& encoder, const Web::Cookie::ParsedCookie& cookie)
+{
+    encoder << cookie.name;
+    encoder << cookie.value;
+    encoder << cookie.expiry_time_from_expires_attribute;
+    encoder << cookie.expiry_time_from_max_age_attribute;
+    encoder << cookie.domain;
+    encoder << cookie.path;
+    encoder << cookie.secure_attribute_present;
+    encoder << cookie.http_only_attribute_present;
+
+    return true;
+}
+
+bool IPC::decode(IPC::Decoder& decoder, Web::Cookie::ParsedCookie& cookie)
+{
+    if (!decoder.decode(cookie.name))
+        return false;
+    if (!decoder.decode(cookie.value))
+        return false;
+    if (!decoder.decode(cookie.expiry_time_from_expires_attribute))
+        return false;
+    if (!decoder.decode(cookie.expiry_time_from_max_age_attribute))
+        return false;
+    if (!decoder.decode(cookie.domain))
+        return false;
+    if (!decoder.decode(cookie.path))
+        return false;
+    if (!decoder.decode(cookie.secure_attribute_present))
+        return false;
+    if (!decoder.decode(cookie.http_only_attribute_present))
+        return false;
+
+    return true;
+}
diff --git a/Userland/Libraries/LibWeb/Cookie/ParsedCookie.h b/Userland/Libraries/LibWeb/Cookie/ParsedCookie.h
index c48103b28e..5d3c8b8020 100644
--- a/Userland/Libraries/LibWeb/Cookie/ParsedCookie.h
+++ b/Userland/Libraries/LibWeb/Cookie/ParsedCookie.h
@@ -29,6 +29,7 @@
 #include <AK/Optional.h>
 #include <AK/String.h>
 #include <LibCore/DateTime.h>
+#include <LibIPC/Forward.h>
 
 namespace Web::Cookie {
 
@@ -46,3 +47,10 @@ struct ParsedCookie {
 Optional<ParsedCookie> parse_cookie(const String& cookie_string);
 
 }
+
+namespace IPC {
+
+bool encode(IPC::Encoder&, const Web::Cookie::ParsedCookie&);
+bool decode(IPC::Decoder&, Web::Cookie::ParsedCookie&);
+
+}
diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp
index 59c2407904..86cb77ab03 100644
--- a/Userland/Libraries/LibWeb/DOM/Document.cpp
+++ b/Userland/Libraries/LibWeb/DOM/Document.cpp
@@ -34,6 +34,7 @@
 #include <LibWeb/Bindings/MainThreadVM.h>
 #include <LibWeb/Bindings/WindowObject.h>
 #include <LibWeb/CSS/StyleResolver.h>
+#include <LibWeb/Cookie/ParsedCookie.h>
 #include <LibWeb/DOM/Comment.h>
 #include <LibWeb/DOM/DOMException.h>
 #include <LibWeb/DOM/Document.h>
@@ -828,10 +829,14 @@ String Document::cookie(Cookie::Source source)
     return {};
 }
 
-void Document::set_cookie(String cookie, Cookie::Source source)
+void Document::set_cookie(String cookie_string, Cookie::Source source)
 {
+    auto cookie = Cookie::parse_cookie(cookie_string);
+    if (!cookie.has_value())
+        return;
+
     if (auto* page = this->page())
-        page->client().page_did_set_cookie(m_url, cookie, source);
+        page->client().page_did_set_cookie(m_url, cookie.value(), source);
 }
 
 }
diff --git a/Userland/Libraries/LibWeb/InProcessWebView.cpp b/Userland/Libraries/LibWeb/InProcessWebView.cpp
index 61cbc087a8..094ef8c8bd 100644
--- a/Userland/Libraries/LibWeb/InProcessWebView.cpp
+++ b/Userland/Libraries/LibWeb/InProcessWebView.cpp
@@ -440,7 +440,7 @@ String InProcessWebView::page_did_request_cookie(const URL& url, Cookie::Source
     return {};
 }
 
-void InProcessWebView::page_did_set_cookie(const URL& url, const String& cookie, Cookie::Source source)
+void InProcessWebView::page_did_set_cookie(const URL& url, const Cookie::ParsedCookie& cookie, Cookie::Source source)
 {
     if (on_set_cookie)
         on_set_cookie(url, cookie, source);
diff --git a/Userland/Libraries/LibWeb/InProcessWebView.h b/Userland/Libraries/LibWeb/InProcessWebView.h
index 46db6e319c..9852abc428 100644
--- a/Userland/Libraries/LibWeb/InProcessWebView.h
+++ b/Userland/Libraries/LibWeb/InProcessWebView.h
@@ -112,7 +112,7 @@ private:
     virtual bool page_did_request_confirm(const String&) override;
     virtual String page_did_request_prompt(const String&, const String&) override;
     virtual String page_did_request_cookie(const URL&, Cookie::Source) override;
-    virtual void page_did_set_cookie(const URL&, const String&, Cookie::Source) override;
+    virtual void page_did_set_cookie(const URL&, const Cookie::ParsedCookie&, Cookie::Source) override;
 
     void layout_and_sync_size();
 
diff --git a/Userland/Libraries/LibWeb/OutOfProcessWebView.cpp b/Userland/Libraries/LibWeb/OutOfProcessWebView.cpp
index 6c069edf69..7e9b5de1f6 100644
--- a/Userland/Libraries/LibWeb/OutOfProcessWebView.cpp
+++ b/Userland/Libraries/LibWeb/OutOfProcessWebView.cpp
@@ -372,7 +372,7 @@ String OutOfProcessWebView::notify_server_did_request_cookie(Badge<WebContentCli
     return {};
 }
 
-void OutOfProcessWebView::notify_server_did_set_cookie(Badge<WebContentClient>, const URL& url, const String& cookie, Cookie::Source source)
+void OutOfProcessWebView::notify_server_did_set_cookie(Badge<WebContentClient>, const URL& url, const Cookie::ParsedCookie& cookie, Cookie::Source source)
 {
     if (on_set_cookie)
         on_set_cookie(url, cookie, source);
diff --git a/Userland/Libraries/LibWeb/OutOfProcessWebView.h b/Userland/Libraries/LibWeb/OutOfProcessWebView.h
index 386cd92be7..d101ebf60f 100644
--- a/Userland/Libraries/LibWeb/OutOfProcessWebView.h
+++ b/Userland/Libraries/LibWeb/OutOfProcessWebView.h
@@ -80,7 +80,7 @@ public:
     void notify_server_did_js_console_output(const String& method, const String& line);
     void notify_server_did_change_favicon(const Gfx::Bitmap& favicon);
     String notify_server_did_request_cookie(Badge<WebContentClient>, const URL& url, Cookie::Source source);
-    void notify_server_did_set_cookie(Badge<WebContentClient>, const URL& url, const String& cookie, Cookie::Source source);
+    void notify_server_did_set_cookie(Badge<WebContentClient>, const URL& url, const Cookie::ParsedCookie& cookie, Cookie::Source source);
 
 private:
     OutOfProcessWebView();
diff --git a/Userland/Libraries/LibWeb/Page/Page.h b/Userland/Libraries/LibWeb/Page/Page.h
index dab43a39a4..e5a62e23cd 100644
--- a/Userland/Libraries/LibWeb/Page/Page.h
+++ b/Userland/Libraries/LibWeb/Page/Page.h
@@ -112,7 +112,7 @@ public:
     virtual bool page_did_request_confirm(const String&) { return false; }
     virtual String page_did_request_prompt(const String&, const String&) { return {}; }
     virtual String page_did_request_cookie(const URL&, Cookie::Source) { return {}; }
-    virtual void page_did_set_cookie(const URL&, const String&, Cookie::Source) { }
+    virtual void page_did_set_cookie(const URL&, const Cookie::ParsedCookie&, Cookie::Source) { }
 
 protected:
     virtual ~PageClient() = default;
diff --git a/Userland/Libraries/LibWeb/WebContentClient.cpp b/Userland/Libraries/LibWeb/WebContentClient.cpp
index dd25e07e3a..c5678a8125 100644
--- a/Userland/Libraries/LibWeb/WebContentClient.cpp
+++ b/Userland/Libraries/LibWeb/WebContentClient.cpp
@@ -27,6 +27,7 @@
 #include "WebContentClient.h"
 #include "OutOfProcessWebView.h"
 #include <AK/Debug.h>
+#include <LibWeb/Cookie/ParsedCookie.h>
 
 namespace Web {
 
diff --git a/Userland/Libraries/LibWeb/WebContentClient.h b/Userland/Libraries/LibWeb/WebContentClient.h
index 54e4a34dbc..320e8793d0 100644
--- a/Userland/Libraries/LibWeb/WebContentClient.h
+++ b/Userland/Libraries/LibWeb/WebContentClient.h
@@ -28,6 +28,7 @@
 
 #include <AK/HashMap.h>
 #include <LibIPC/ServerConnection.h>
+#include <LibWeb/Cookie/ParsedCookie.h>
 #include <WebContent/WebContentClientEndpoint.h>
 #include <WebContent/WebContentServerEndpoint.h>
 
diff --git a/Userland/Libraries/LibWeb/WebViewHooks.h b/Userland/Libraries/LibWeb/WebViewHooks.h
index a8f4d2911c..8cb1936a8f 100644
--- a/Userland/Libraries/LibWeb/WebViewHooks.h
+++ b/Userland/Libraries/LibWeb/WebViewHooks.h
@@ -49,7 +49,7 @@ public:
     Function<void(const URL&, const String&)> on_get_source;
     Function<void(const String& method, const String& line)> on_js_console_output;
     Function<String(const URL& url, Cookie::Source source)> on_get_cookie;
-    Function<void(const URL& url, const String& cookie, Cookie::Source source)> on_set_cookie;
+    Function<void(const URL& url, const Cookie::ParsedCookie& cookie, Cookie::Source source)> on_set_cookie;
 };
 
 }
diff --git a/Userland/Services/WebContent/ClientConnection.cpp b/Userland/Services/WebContent/ClientConnection.cpp
index 8cc22ce3c2..7d2214497c 100644
--- a/Userland/Services/WebContent/ClientConnection.cpp
+++ b/Userland/Services/WebContent/ClientConnection.cpp
@@ -34,6 +34,7 @@
 #include <LibJS/Parser.h>
 #include <LibJS/Runtime/VM.h>
 #include <LibWeb/Bindings/MainThreadVM.h>
+#include <LibWeb/Cookie/ParsedCookie.h>
 #include <LibWeb/DOM/Document.h>
 #include <LibWeb/Dump.h>
 #include <LibWeb/Layout/InitialContainingBlockBox.h>
diff --git a/Userland/Services/WebContent/ClientConnection.h b/Userland/Services/WebContent/ClientConnection.h
index 3fd0a3c662..4027db85d7 100644
--- a/Userland/Services/WebContent/ClientConnection.h
+++ b/Userland/Services/WebContent/ClientConnection.h
@@ -29,6 +29,7 @@
 #include <AK/HashMap.h>
 #include <LibIPC/ClientConnection.h>
 #include <LibJS/Forward.h>
+#include <LibWeb/Cookie/ParsedCookie.h>
 #include <LibWeb/Forward.h>
 #include <WebContent/Forward.h>
 #include <WebContent/WebContentClientEndpoint.h>
diff --git a/Userland/Services/WebContent/PageHost.cpp b/Userland/Services/WebContent/PageHost.cpp
index f7ae26e18f..11e074c3b0 100644
--- a/Userland/Services/WebContent/PageHost.cpp
+++ b/Userland/Services/WebContent/PageHost.cpp
@@ -28,6 +28,7 @@
 #include "ClientConnection.h"
 #include <LibGfx/Painter.h>
 #include <LibGfx/SystemTheme.h>
+#include <LibWeb/Cookie/ParsedCookie.h>
 #include <LibWeb/Layout/InitialContainingBlockBox.h>
 #include <LibWeb/Page/Frame.h>
 #include <WebContent/WebContentClientEndpoint.h>
@@ -213,7 +214,7 @@ String PageHost::page_did_request_cookie(const URL& url, Web::Cookie::Source sou
     return m_client.send_sync<Messages::WebContentClient::DidRequestCookie>(url, static_cast<u8>(source))->cookie();
 }
 
-void PageHost::page_did_set_cookie(const URL& url, const String& cookie, Web::Cookie::Source source)
+void PageHost::page_did_set_cookie(const URL& url, const Web::Cookie::ParsedCookie& cookie, Web::Cookie::Source source)
 {
     m_client.post_message(Messages::WebContentClient::DidSetCookie(url, cookie, static_cast<u8>(source)));
 }
diff --git a/Userland/Services/WebContent/PageHost.h b/Userland/Services/WebContent/PageHost.h
index 6872a51a13..eda03a669d 100644
--- a/Userland/Services/WebContent/PageHost.h
+++ b/Userland/Services/WebContent/PageHost.h
@@ -80,7 +80,7 @@ private:
     virtual void page_did_change_favicon(const Gfx::Bitmap&) override;
     virtual void page_did_request_image_context_menu(const Gfx::IntPoint&, const URL&, const String& target, unsigned modifiers, const Gfx::Bitmap*) override;
     virtual String page_did_request_cookie(const URL&, Web::Cookie::Source) override;
-    virtual void page_did_set_cookie(const URL&, const String&, Web::Cookie::Source) override;
+    virtual void page_did_set_cookie(const URL&, const Web::Cookie::ParsedCookie&, Web::Cookie::Source) override;
 
     explicit PageHost(ClientConnection&);
 
diff --git a/Userland/Services/WebContent/WebContentClient.ipc b/Userland/Services/WebContent/WebContentClient.ipc
index a93fd8ed7c..c74abbc3af 100644
--- a/Userland/Services/WebContent/WebContentClient.ipc
+++ b/Userland/Services/WebContent/WebContentClient.ipc
@@ -26,5 +26,5 @@ endpoint WebContentClient = 90
     DidJSConsoleOutput(String method, String line) =|
     DidChangeFavicon(Gfx::ShareableBitmap favicon) =|
     DidRequestCookie(URL url, u8 source) => (String cookie)
-    DidSetCookie(URL url, String cookie, u8 source) =|
+    DidSetCookie(URL url, Web::Cookie::ParsedCookie cookie, u8 source) =|
 }