From 03c1c21fd3e8369363fcf86c37d3fbe3b78d40fb Mon Sep 17 00:00:00 2001
From: Andrei Kurushin <ajax16384@gmail.com>
Date: Wed, 8 May 2019 18:12:21 +0000
Subject: [PATCH] Fix crash when setting an invalid cookie (fixes issue #2657)

---
 libcef/browser/net/cookie_manager_old_impl.cc | 24 ++++++++++-----
 .../net_service/cookie_manager_impl.cc        |  5 ++++
 tests/ceftests/cookie_unittest.cc             | 29 +++++++++++++++++++
 3 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/libcef/browser/net/cookie_manager_old_impl.cc b/libcef/browser/net/cookie_manager_old_impl.cc
index 1b2489095..fb07a3a0b 100644
--- a/libcef/browser/net/cookie_manager_old_impl.cc
+++ b/libcef/browser/net/cookie_manager_old_impl.cc
@@ -400,18 +400,26 @@ void CefCookieManagerOldImpl::SetCookieInternal(
   if (cookie.has_expires)
     cef_time_to_basetime(cookie.expires, expiration_time);
 
+  auto canonical_cookie = net::CanonicalCookie::CreateSanitizedCookie(
+      url, name, value, domain, path,
+      base::Time(),  // Creation time.
+      expiration_time,
+      base::Time(),  // Last access time.
+      cookie.secure ? true : false, cookie.httponly ? true : false,
+      net::CookieSameSite::DEFAULT_MODE, net::COOKIE_PRIORITY_DEFAULT);
+
   net::CookieOptions options;
   if (cookie.httponly)
     options.set_include_httponly();
 
-  cookie_store->SetCanonicalCookieAsync(
-      net::CanonicalCookie::CreateSanitizedCookie(
-          url, name, value, domain, path,
-          base::Time(),  // Creation time.
-          expiration_time,
-          base::Time(),  // Last access time.
-          cookie.secure ? true : false, cookie.httponly ? true : false,
-          net::CookieSameSite::DEFAULT_MODE, net::COOKIE_PRIORITY_DEFAULT),
+  if (!canonical_cookie) {
+    SetCookieCallbackImpl(
+        callback,
+        net::CanonicalCookie::CookieInclusionStatus::EXCLUDE_UNKNOWN_ERROR);
+    return;
+  }
+
+  cookie_store->SetCanonicalCookieAsync(std::move(canonical_cookie),
       url.scheme(), options, base::Bind(SetCookieCallbackImpl, callback));
 }
 
diff --git a/libcef/browser/net_service/cookie_manager_impl.cc b/libcef/browser/net_service/cookie_manager_impl.cc
index a80b87f07..91ab07b84 100644
--- a/libcef/browser/net_service/cookie_manager_impl.cc
+++ b/libcef/browser/net_service/cookie_manager_impl.cc
@@ -209,6 +209,11 @@ bool CefCookieManagerImpl::SetCookie(const CefString& url,
       cookie.secure ? true : false, cookie.httponly ? true : false,
       net::CookieSameSite::DEFAULT_MODE, net::COOKIE_PRIORITY_DEFAULT);
 
+  if (!canonical_cookie) {
+    SetCookieCallbackImpl(callback, false);
+    return true;
+  }
+
   net::CookieOptions options;
   if (cookie.httponly)
     options.set_include_httponly();
diff --git a/tests/ceftests/cookie_unittest.cc b/tests/ceftests/cookie_unittest.cc
index f8956fcd1..3218fb193 100644
--- a/tests/ceftests/cookie_unittest.cc
+++ b/tests/ceftests/cookie_unittest.cc
@@ -300,6 +300,22 @@ void TestHostCookie(CefRefPtr<CefCookieManager> manager,
   VerifyNoCookies(manager, event, true);
 }
 
+void TestInvalidCookie(CefRefPtr<CefCookieManager> manager,
+                       CefRefPtr<CefWaitableEvent> event) {
+  CookieVector cookies;
+
+  CefCookie cookie;
+  const char* kUrl = "http://www.xyz.com";
+  CefString(&cookie.name).FromASCII("invalid1");
+  CefString(&cookie.value).FromASCII("invalid1");
+  CefString(&cookie.domain).FromASCII(".zyx.com");  // domain mismatch
+
+  cookies.push_back(cookie);
+
+  // No cookies will be set due to non canonical cookie
+  SetCookies(manager, kUrl, cookies, false, event);
+}
+
 void TestMultipleCookies(CefRefPtr<CefCookieManager> manager,
                          CefRefPtr<CefWaitableEvent> event) {
   std::stringstream ss;
@@ -474,6 +490,19 @@ void TestAllCookies(CefRefPtr<CefCookieManager> manager,
 
 }  // namespace
 
+// Test creation of a invalid cookie.
+TEST(CookieTest, BasicInvalidCookie) {
+  CefRefPtr<CefWaitableEvent> event =
+      CefWaitableEvent::CreateWaitableEvent(true, false);
+
+  CefRefPtr<CefCookieManager> manager =
+      CefCookieManager::GetGlobalManager(new TestCompletionCallback(event));
+  event->Wait();
+  EXPECT_TRUE(manager.get());
+
+  TestInvalidCookie(manager, event);
+}
+
 // Test creation of a domain cookie.
 TEST(CookieTest, BasicDomainCookie) {
   CefRefPtr<CefWaitableEvent> event =