Fix some issues detected by the testsuite

* An empty "expires" field results in a session cookie, rather than
rejecting the cookie altogether
* A page can set a cookie it is not allowed to access (for example in a
subdirectory of where the page is located). Separate IsValidForUrl and
_CanBeSetFromUrl to perform the appropriate checks in each case.
* Limit cookie path to 4096 characters. As a result of the previous
change, a page would be allowed to set a cookie with an aribrarily long
subpath, wasting disk space and RAM by growing hte cookie jar.
* Don't allow path with . or .. elements. These are a source of
confusion and are not needed.
* Reset the cookie fields when parsing failed. This does not matter when
using the cookie jar, but is useful when working directly with
BNetworkCookie.
This commit is contained in:
Adrien Destugues 2014-06-06 16:53:50 +02:00
parent fed9b96e26
commit 550f5b1c95

View File

@ -161,7 +161,7 @@ BNetworkCookie::ParseCookieString(const BString& string, const BUrl& url)
SetMaxAge(-1); // cookie will expire immediately
} else if (name.ICompare("expires") == 0) {
if (value.IsEmpty()) {
result = B_BAD_VALUE;
// Will be a session cookie.
continue;
}
BDateTime parsed = BHttpTime(value).Parse();
@ -187,11 +187,11 @@ BNetworkCookie::ParseCookieString(const BString& string, const BUrl& url)
}
}
if (!IsValidForUrl(url)) {
// Invalidate the cookie.
if (!_CanBeSetFromUrl(url))
result = B_NOT_ALLOWED;
if (result != B_OK)
_Reset();
return B_NOT_ALLOWED;
}
return result;
}
@ -221,14 +221,22 @@ BNetworkCookie::SetValue(const BString& value)
status_t
BNetworkCookie::SetPath(const BString& path)
BNetworkCookie::SetPath(const BString& to)
{
if (path[0] != '/')
fPath.Truncate(0);
fRawFullCookieValid = false;
// Limit the path to 4096 characters to not let the cookie jar grow huge.
if (to[0] != '/' || to.Length() > 4096)
return B_BAD_DATA;
// TODO: canonicalize the path
fPath = path;
fRawFullCookieValid = false;
// Check that there aren't any "." or ".." segments in the path.
if (to.EndsWith("/.") || to.EndsWith("/.."))
return B_BAD_DATA;
if (to.FindFirst("/../") >= 0 || to.FindFirst("/./") >= 0)
return B_BAD_DATA;
fPath = to;
return B_OK;
}
@ -286,15 +294,14 @@ BNetworkCookie::SetExpirationDate(BDateTime& expireDate)
if (!expireDate.IsValid()) {
fExpiration.SetTime_t(0);
fSessionCookie = true;
fExpirationStringValid = false;
fRawFullCookieValid = false;
} else {
fExpiration = expireDate;
fSessionCookie = false;
fExpirationStringValid = false;
fRawFullCookieValid = false;
}
fExpirationStringValid = false;
fRawFullCookieValid = false;
return *this;
}
@ -490,7 +497,6 @@ BNetworkCookie::IsValidForPath(const BString& path) const
{
const BString& cookiePath = Path();
BString normalizedPath = path;
int slashPos = normalizedPath.FindLast('/');
if (slashPos != normalizedPath.Length() - 1)
normalizedPath.Truncate(slashPos + 1);
@ -503,6 +509,34 @@ BNetworkCookie::IsValidForPath(const BString& path) const
}
bool
BNetworkCookie::_CanBeSetFromUrl(const BUrl& url) const
{
if (Secure() && url.Protocol() != "https")
return false;
if (url.Protocol() == "file")
return Domain() == "localhost" && _CanBeSetFromPath(url.Path());
return IsValidForDomain(url.Host()) && _CanBeSetFromPath(url.Path());
}
bool
BNetworkCookie::_CanBeSetFromPath(const BString& path) const
{
BString normalizedPath = path;
int slashPos = normalizedPath.FindLast('/');
normalizedPath.Truncate(slashPos);
if (Path().Compare(normalizedPath, normalizedPath.Length()) == 0)
return true;
else if (normalizedPath.Compare(Path(), Path().Length()) == 0)
return true;
return false;
}
// #pragma mark Cookie fields existence tests