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.

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);
     }

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();
 

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; }

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;
+}

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&);
+
+}

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);
 }
 
 }

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);

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();
 

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);

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();

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;

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 {
 

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>
 

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;
 };
 
 }

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>

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>

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)));
 }

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&);
 

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) =|
 }