Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(609)

Unified Diff: ios/net/cookies/cookie_store_ios.mm

Issue 2861063003: Remove dangerous CanonicalCookie::Create method. (Closed)
Patch Set: Moved test to shared file. Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « ios/net/cookies/cookie_cache_unittest.cc ('k') | ios/net/cookies/cookie_store_ios_test_util.mm » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: ios/net/cookies/cookie_store_ios.mm
diff --git a/ios/net/cookies/cookie_store_ios.mm b/ios/net/cookies/cookie_store_ios.mm
index 8e9e9ea2c4ce0d4b66dd3589f138341006f6a679..30c7180bd835d2f0172573db7f856acc399fd236 100644
--- a/ios/net/cookies/cookie_store_ios.mm
+++ b/ios/net/cookies/cookie_store_ios.mm
@@ -374,12 +374,45 @@ void CookieStoreIOS::SetCookieWithDetailsAsync(
if (creation_time.is_null())
creation_time = base::Time::Now();
+ // Validate consistency of passed arguments.
+ if (ParsedCookie::ParseTokenString(name) != name ||
+ ParsedCookie::ParseValueString(value) != value ||
+ ParsedCookie::ParseValueString(domain) != domain ||
+ ParsedCookie::ParseValueString(path) != path) {
+ if (!callback.is_null())
+ callback.Run(false);
+ return;
+ }
+
+ // Validate passed arguments against URL.
+ std::string cookie_domain;
+ std::string cookie_path = CanonicalCookie::CanonPathWithString(url, path);
+ if ((secure && !url.SchemeIsCryptographic()) ||
+ !cookie_util::GetCookieDomainWithString(url, domain, &cookie_domain) ||
+ (!path.empty() && cookie_path != path)) {
+ if (!callback.is_null())
+ callback.Run(false);
+ return;
+ }
+
+ // Canonicalize path again to make sure it escapes characters as needed.
+ url::Component path_component(0, cookie_path.length());
+ url::RawCanonOutputT<char> canon_path;
+ url::Component canon_path_component;
+ url::CanonicalizePath(cookie_path.data(), path_component, &canon_path,
+ &canon_path_component);
+ cookie_path = std::string(canon_path.data() + canon_path_component.begin,
+ canon_path_component.len);
+
// First create a CanonicalCookie, to normalize the arguments,
// particularly domain and path, and perform validation.
std::unique_ptr<net::CanonicalCookie> canonical_cookie =
net::CanonicalCookie::Create(
- url, name, value, domain, path, creation_time, expiration_time,
- secure, http_only, same_site, priority);
+ name, value, cookie_domain, cookie_path, creation_time,
+ // TODO(rdsmith): Check with ios reviewer about whether to use
Elly Fong-Jones 2017/05/11 17:24:35 I would use creation_time, which is what the old :
Randy Smith (Not in Mondays) 2017/05/12 17:02:02 Done.
+ // last_access_time here. Wasn't used in original implementation.
+ expiration_time, base::Time(), secure, http_only, same_site,
+ priority);
if (canonical_cookie) {
NSHTTPCookie* cookie = SystemCookieFromCanonicalCookie(*canonical_cookie);
« no previous file with comments | « ios/net/cookies/cookie_cache_unittest.cc ('k') | ios/net/cookies/cookie_store_ios_test_util.mm » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698