Skip to content

Commit

Permalink
Adding checking for max expiriation date in IE driver cookie handling
Browse files Browse the repository at this point in the history
Attempting to add a cookie with a very large expiration date caused
the IE driver to crash because of overflows in the C++ standard library
time formatting functions. This commit now avoids the crash by
enforcing a maximum cookie expiration time of 2,147,483,647 (2^32 - 1)
seconds from the time the cookie is attempting to be set. At the time
of this writing (2019), that means an expiration of the cookie sometime
over 68 years into the future (2087). If the intended expiration time
is beyond that limit, the driver will now return "unable to set cookie"
error.

As an internal implementation detail, the driver is migrating from the
"expires" attribute to the "max-age" attribute of the cookie string,
as versions of IE below 9 are no longer supported. Additionally, while
it would be possible to use "expires" and get a longer expiration time,
(somewhere around the year 3000), the distinction between a cookie
expiring after 68 years and one expiring in just under 1000 years is
(or ought to be) largely meaningless in the context of a website.

Fixes issue #7122.
  • Loading branch information
jimevans committed Apr 22, 2019
1 parent 9cbc45b commit 103f12a
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 16 deletions.
13 changes: 5 additions & 8 deletions cpp/iedriver/BrowserCookie.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,10 @@ std::string BrowserCookie::ToString() const {

if (this->expiration_time_ > 0) {
time_t expiration_time = static_cast<time_t>(this->expiration_time_);
std::vector<char> raw_formatted_time(30);
tm time_info;
gmtime_s(&time_info, &expiration_time);
std::string format_string = "%a, %d %b %Y %H:%M:%S GMT";
strftime(&raw_formatted_time[0], 30, format_string.c_str(), &time_info);
std::string formatted_time(&raw_formatted_time[0]);
cookie_string += "expires=" + formatted_time + "; ";
time_t current_time;
time(&current_time);
long long expiration_seconds = expiration_time - current_time;
cookie_string += "max-age=" + std::to_string(expiration_seconds) + ";";
}

if (this->domain_.size() > 0) {
Expand Down Expand Up @@ -118,4 +115,4 @@ BrowserCookie BrowserCookie::Copy(void) const {
return destination_cookie;
}

}
}
23 changes: 23 additions & 0 deletions cpp/iedriver/CommandHandlers/AddCookieCommandHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
#include "../CookieManager.h"
#include "../IECommandExecutor.h"

#define MAX_EXPIRATION_SECONDS 2147483647

namespace webdriver {

AddCookieCommandHandler::AddCookieCommandHandler(void) {
Expand All @@ -43,6 +45,27 @@ void AddCookieCommandHandler::ExecuteInternal(
Json::Value cookie_value = cookie_parameter_iterator->second;
BrowserCookie cookie = BrowserCookie::FromJson(cookie_value);

if (cookie.expiration_time() > MAX_EXPIRATION_SECONDS) {
time_t current_time;
time(&current_time);
time_t max_time = current_time + MAX_EXPIRATION_SECONDS;
std::vector<char> raw_formatted_time(30);
tm time_info;
gmtime_s(&time_info, &max_time);
std::string format_string = "%a, %d %b %Y %H:%M:%S GMT";
strftime(&raw_formatted_time[0], 30, format_string.c_str(), &time_info);
std::string formatted_time(&raw_formatted_time[0]);

std::string error_message = "Internet Explorer does not allow cookies to ";
error_message.append("be set more than ");
error_message.append(std::to_string(MAX_EXPIRATION_SECONDS)).append(" ");
error_message.append("(2 ^ 32 - 1) seconds into the future, or ");
error_message.append(formatted_time).append(". This ia a limitaton of ");
error_message.append("the browser, not the driver.");
response->SetErrorResponse(ERROR_UNABLE_TO_SET_COOKIE, error_message);
return;
}

BrowserHandle browser_wrapper;
int status_code = executor.GetCurrentBrowser(&browser_wrapper);
if (status_code != WD_SUCCESS) {
Expand Down
8 changes: 4 additions & 4 deletions cpp/iedriver/IEDriver.rc
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ END
//

VS_VERSION_INFO VERSIONINFO
FILEVERSION 3,141,59,0
PRODUCTVERSION 3,141,59,0
FILEVERSION 3,141,59,1
PRODUCTVERSION 3,141,59,1
FILEFLAGSMASK 0x3fL
#ifdef _DEBUG
FILEFLAGS 0x1L
Expand All @@ -68,12 +68,12 @@ BEGIN
BEGIN
VALUE "CompanyName", "Software Freedom Conservancy"
VALUE "FileDescription", "Driver library for the IE driver"
VALUE "FileVersion", "3.141.59.0"
VALUE "FileVersion", "3.141.59.1"
VALUE "InternalName", "IEDriver.dll"
VALUE "LegalCopyright", "Copyright (C) 2019"
VALUE "OriginalFilename", "IEDriver.dll"
VALUE "ProductName", "Selenium WebDriver"
VALUE "ProductVersion", "3.141.59.0"
VALUE "ProductVersion", "3.141.59.1"
END
END
BLOCK "VarFileInfo"
Expand Down
22 changes: 22 additions & 0 deletions cpp/iedriverserver/CHANGELOG
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,28 @@ available via the project downloads page. Changes in "revision" field indicate
private releases checked into the prebuilts directory of the source tree, but
not made generally available on the downloads page.

v3.151.59.1
===========
* Added checking for max expiriation date in IE driver cookie handling.
Attempting to add a cookie with a very large expiration date caused
the IE driver to crash because of overflows in the C++ standard library
time formatting functions. This commit now avoids the crash by
enforcing a maximum cookie expiration time of 2,147,483,647 (2^32 - 1)
seconds from the time the cookie is attempting to be set. At the time
of this writing (2019), that means an expiration of the cookie sometime
over 68 years into the future (2087). If the intended expiration time
is beyond that limit, the driver will now return "unable to set cookie"
error.

As an internal implementation detail, the driver is migrating from the
"expires" attribute to the "max-age" attribute of the cookie string,
as versions of IE below 9 are no longer supported. Additionally, while
it would be possible to use "expires" and get a longer expiration time,
(somewhere around the year 3000), the distinction between a cookie
expiring after 68 years and one expiring in just under 1000 years is
(or ought to be) largely meaningless in the context of a website.
Fixes issue #7122.

v3.151.59.0
===========
* Rollup of fixes since previous release. No additional changes.
Expand Down
8 changes: 4 additions & 4 deletions cpp/iedriverserver/IEDriverServer.rc
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ END
//

VS_VERSION_INFO VERSIONINFO
FILEVERSION 3,141,59,0
PRODUCTVERSION 3,141,59,0
FILEVERSION 3,141,59,1
PRODUCTVERSION 3,141,59,1
FILEFLAGSMASK 0x3fL
#ifdef _DEBUG
FILEFLAGS 0x1L
Expand All @@ -68,12 +68,12 @@ BEGIN
BEGIN
VALUE "CompanyName", "Software Freedom Conservancy"
VALUE "FileDescription", "Command line server for the IE driver"
VALUE "FileVersion", "3.141.59.0"
VALUE "FileVersion", "3.141.59.1"
VALUE "InternalName", "IEDriverServer.exe"
VALUE "LegalCopyright", "Copyright (C) 2019"
VALUE "OriginalFilename", "IEDriverServer.exe"
VALUE "ProductName", "Selenium WebDriver"
VALUE "ProductVersion", "3.141.59.0"
VALUE "ProductVersion", "3.141.59.1"
END
END
BLOCK "VarFileInfo"
Expand Down
Binary file modified cpp/prebuilt/Win32/Release/IEDriverServer.exe
Binary file not shown.
Binary file modified cpp/prebuilt/x64/Release/IEDriverServer.exe
Binary file not shown.

0 comments on commit 103f12a

Please sign in to comment.