Skip to content

Commit

Permalink
Improving IE driver use with invalid Protected Mode settings
Browse files Browse the repository at this point in the history
Now, when the user does not set the Protected Mode settings of the
browser and sends the capability to bypass the checks for those
settings, the driver will attempt to predict when a Protected Mode
boundary will be crossed, and set in motion a process to reattach
itself to the newly created browser. This process is far from perfect.
It is subject to really challenging race conditions that are truly
impossible to eliminate entirely, because of the architecture of the
browser itself. Nevertheless, even in its flawed state, this is still
a better outcome than it was previously for users.

Please note that the advice and support policy of the IE driver will
continue to be that the user must set the Protected Mode settings of
the browser properly before using the driver. Any "issues" that arise
by not having the settings set, but that disappear when the settings
are corrected, are not considered by the project to be valid issues.
This will include, but not be limited to, issues like abandoned
browser instances not being closed, use of multiple instances of the
driver where the wrong browser window is connected to and automated,
and issues where the driver appears to hang upon navigation to a new
page. If the problem disappears when the browser is properly
configured, any issue reports will be immediately closed with a note
to properly configure the browser and remove the capability.

The following situations should be at least partially mitigated by the
change:

 * Navigation to a new page
 * Clicking on a link (specifically an <a> tag) that will lead to a
   navigation to a new page
 * Clicking a link that opens a new window

Other cases, like navigating backward and forward through the browser
history, clicking an element that submits a form, and so on, may not
be handled. In those cases, issue reports will be summarily closed,
unless a specific pull request fixing the issue is also provided.
Additionally, use of things like proxies to capture traffic between the
browser and web server may miss some traffic because of the race
conditions inherent in the mechanism used to reattach to a newly
created browser. Again, these race conditions are unavoidable, and
issue reports that are based on them will be immediately closed with a
note indicating that the browser must have its settings properly set.
These strict guidelines are not intended to be harsh, and are not put
in place with the intent to avoid investigating and fixing issues;
rather, they must be enforced because the underlying architecture of
the browser makes them unavoidable.
  • Loading branch information
jimevans committed Feb 17, 2019
1 parent 4080dc8 commit 91ea70d
Show file tree
Hide file tree
Showing 23 changed files with 563 additions and 68 deletions.
155 changes: 141 additions & 14 deletions cpp/iedriver/Browser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "Browser.h"

#include <comutil.h>
#include <IEPMapi.h>
#include <ShlGuid.h>

#include "errorcodes.h"
Expand All @@ -26,9 +27,11 @@
#include "BrowserFactory.h"
#include "CustomTypes.h"
#include "messages.h"
#include "HookProcessor.h"
#include "Script.h"
#include "StringUtilities.h"
#include "WebDriverConstants.h"
#include "WindowUtilities.h"

namespace webdriver {

Expand All @@ -52,24 +55,58 @@ void __stdcall Browser::BeforeNavigate2(IDispatch* pObject,
VARIANT* pvarHeaders,
VARIANT_BOOL* pbCancel) {
LOG(TRACE) << "Entering Browser::BeforeNavigate2";
std::wstring url(pvarUrl->bstrVal);

LOG(DEBUG) << "BeforeNavigate2: Url: " << LOGWSTRING(url) << ", TargetFrame: " << pvarTargetFrame->bstrVal;
}

void __stdcall Browser::OnQuit() {
LOG(TRACE) << "Entering Browser::OnQuit";
if (!this->is_explicit_close_requested_) {
LOG(WARN) << "This instance of Internet Explorer is exiting without an "
<< "explicit request to close it. Unless you clicked a link "
<< "that specifically attempts to close the page, that likely "
<< "means a Protected Mode boundary has been crossed (either "
<< "entering or exiting Protected Mode). It is highly likely "
<< "that any subsequent commands to this driver instance will "
<< "fail. THIS IS NOT A BUG IN THE IE DRIVER! Fix your code "
<< "and/or browser configuration so that a Protected Mode "
<< "boundary is not crossed.";
if (this->is_awaiting_new_process()) {
LOG(WARN) << "A new browser process was requested. This means a Protected "
<< "Mode boundary has been crossed, and that future commands to "
<< "the current browser instance will fail. The driver will "
<< "attempt to reconnect to the newly created browser object, "
<< "but there is no guarantee it will work.";
DWORD process_id;
HWND window_handle = this->GetBrowserWindowHandle();
::GetWindowThreadProcessId(window_handle, &process_id);

BrowserReattachInfo* info = new BrowserReattachInfo;
info->browser_id = this->browser_id();
info->current_process_id = process_id;
info->known_process_ids = this->known_process_ids_;

this->DetachEvents();
this->browser_ = NULL;
::PostMessage(this->executor_handle(),
WD_BROWSER_REATTACH,
NULL,
reinterpret_cast<LPARAM>(info));
return;
} else {
LOG(WARN) << "This instance of Internet Explorer is exiting without an "
<< "explicit request to close it. Unless you clicked a link "
<< "that specifically attempts to close the page, that likely "
<< "means a Protected Mode boundary has been crossed (either "
<< "entering or exiting Protected Mode). It is highly likely "
<< "that any subsequent commands to this driver instance will "
<< "fail. THIS IS NOT A BUG IN THE IE DRIVER! Fix your code "
<< "and/or browser configuration so that a Protected Mode "
<< "boundary is not crossed.";
}
}
this->PostQuitMessage();
}

void __stdcall Browser::NewProcess(DWORD lCauseFlag,
IDispatch* pWB2,
VARIANT_BOOL* pbCancel) {
LOG(TRACE) << "Entering Browser::NewProcess";
this->InitiateBrowserReattach();
}

void __stdcall Browser::NewWindow3(IDispatch** ppDisp,
VARIANT_BOOL* pbCancel,
DWORD dwFlags,
Expand All @@ -84,14 +121,15 @@ void __stdcall Browser::NewWindow3(IDispatch** ppDisp,
// This will not allow us to handle windows created by the JavaScript
// showModalDialog function().
::PostMessage(this->executor_handle(), WD_BEFORE_NEW_WINDOW, NULL, NULL);
std::wstring url = bstrUrl;
IWebBrowser2* browser;
LPSTREAM message_payload;
NewWindowInfo info;
info.target_url = StringUtilities::ToString(url);
LRESULT create_result = ::SendMessage(this->executor_handle(),
WD_BROWSER_NEW_WINDOW,
NULL,
reinterpret_cast<LPARAM>(&message_payload));
reinterpret_cast<LPARAM>(&info));
if (create_result != 0) {

// The new, blank IWebBrowser2 object was not created,
// so we can't really do anything appropriate here.
// Note this is "response method 2" of the aforementioned
Expand All @@ -104,7 +142,7 @@ void __stdcall Browser::NewWindow3(IDispatch** ppDisp,

// We received a valid IWebBrowser2 pointer, so deserialize it onto this
// thread, and pass the result back to the caller.
HRESULT hr = ::CoGetInterfaceAndReleaseStream(message_payload,
HRESULT hr = ::CoGetInterfaceAndReleaseStream(info.browser_stream,
IID_IWebBrowser2,
reinterpret_cast<void**>(&browser));
if (FAILED(hr)) {
Expand Down Expand Up @@ -139,6 +177,73 @@ void __stdcall Browser::DocumentComplete(IDispatch* pDisp, VARIANT* URL) {
}
}

bool Browser::IsCrossZoneUrl(std::string url) {
LOG(TRACE) << "Entering Browser::IsCrossZoneUrl";
std::wstring target_url = StringUtilities::ToWString(url);
CComPtr<IUri> parsed_url;
HRESULT hr = ::CreateUri(target_url.c_str(),
Uri_CREATE_IE_SETTINGS,
0,
&parsed_url);
if (FAILED(hr)) {
// If we can't parse the URL, assume that it's invalid, and
// therefore won't cross a Protected Mode boundary.
return false;
}
bool is_protected_mode_browser = this->IsProtectedMode();
bool is_protected_mode_url = is_protected_mode_browser;
if (url != "about:blank") {
is_protected_mode_url = ::IEIsProtectedModeURL(target_url.c_str()) == S_OK;
}
bool is_cross_zone = is_protected_mode_browser != is_protected_mode_url;
if (is_cross_zone) {
LOG(DEBUG) << "Navigation across Protected Mode zone detected. URL: " << url
<< ", is URL Protected Mode: " << (is_protected_mode_url ? "true" : "false")
<< ", is IE in Protected Mode: " << (is_protected_mode_browser ? "true" : "false");
}
return is_cross_zone;
}

bool Browser::IsProtectedMode() {
LOG(TRACE) << "Entering Browser::IsProtectedMode";
HWND window_handle = this->GetBrowserWindowHandle();
HookSettings hook_settings;
hook_settings.hook_procedure_name = "ProtectedModeWndProc";
hook_settings.hook_procedure_type = WH_CALLWNDPROC;
hook_settings.window_handle = window_handle;
hook_settings.communication_type = OneWay;

HookProcessor hook;
if (!hook.CanSetWindowsHook(window_handle)) {
LOG(WARN) << "Cannot check Protected Mode because driver and browser are "
<< "not the same bit-ness.";
return false;
}
hook.Initialize(hook_settings);
HookProcessor::ResetFlag();
::SendMessage(window_handle, WD_IS_BROWSER_PROTECTED_MODE, NULL, NULL);
bool is_protected_mode = HookProcessor::GetFlagValue();
return is_protected_mode;
}

void Browser::InitiateBrowserReattach() {
LOG(TRACE) << "Entering Browser::InitiateBrowserReattach";
LOG(DEBUG) << "Requesting browser reattach";
this->known_process_ids_.clear();
WindowUtilities::GetProcessesByName(L"iexplore.exe",
&this->known_process_ids_);
this->set_is_awaiting_new_process(true);
::SendMessage(this->executor_handle(), WD_BEFORE_BROWSER_REATTACH, NULL, NULL);
}

void Browser::ReattachBrowser(IWebBrowser2* browser) {
LOG(TRACE) << "Entering Browser::ReattachBrowser";
this->browser_ = browser;
this->AttachEvents();
this->set_is_awaiting_new_process(false);
LOG(DEBUG) << "Reattach complete";
}

void Browser::GetDocument(IHTMLDocument2** doc) {
this->GetDocument(false, doc);
}
Expand Down Expand Up @@ -475,13 +580,17 @@ bool Browser::IsBusy() {

bool Browser::Wait(const std::string& page_load_strategy) {
LOG(TRACE) << "Entering Browser::Wait";

if (page_load_strategy == NONE_PAGE_LOAD_STRATEGY) {
LOG(DEBUG) << "Page load strategy is 'none'. Aborting wait.";
this->set_wait_required(false);
return true;
}

if (this->is_awaiting_new_process()) {
return false;
}

bool is_navigating = true;

LOG(DEBUG) << "Navigate Events Completed.";
Expand Down Expand Up @@ -799,3 +908,21 @@ void Browser::CheckDialogType(HWND dialog_window_handle) {
}

} // namespace webdriver

#ifdef __cplusplus
extern "C" {
#endif

LRESULT CALLBACK ProtectedModeWndProc(int nCode, WPARAM wParam, LPARAM lParam) {
CWPSTRUCT* call_window_proc_struct = reinterpret_cast<CWPSTRUCT*>(lParam);
if (WD_IS_BROWSER_PROTECTED_MODE == call_window_proc_struct->message) {
BOOL is_protected_mode = FALSE;
HRESULT hr = ::IEIsProtectedModeProcess(&is_protected_mode);
webdriver::HookProcessor::SetFlagValue(is_protected_mode == TRUE);
}
return ::CallNextHookEx(NULL, nCode, wParam, lParam);
}

#ifdef __cplusplus
}
#endif
29 changes: 29 additions & 0 deletions cpp/iedriver/Browser.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#define WEBDRIVER_IE_BROWSER_H_

#include <string>
#include <vector>

#include <exdispid.h>
#include <mshtml.h>
Expand All @@ -26,6 +27,17 @@

namespace webdriver {

struct BrowserReattachInfo {
DWORD current_process_id;
std::vector<DWORD> known_process_ids;
std::string browser_id;
};

struct NewWindowInfo {
std::string target_url;
LPSTREAM browser_stream;
};

// Forward declaration of classes to avoid
// circular include files.
class ElementRepository;
Expand Down Expand Up @@ -73,18 +85,28 @@ class Browser : public DocumentHost, public IDispEventSimpleImpl<1, Browser, &DI
return &kNewWindow3;
}

static inline _ATL_FUNC_INFO* NewProcessInfo() {
static _ATL_FUNC_INFO kNewProcess = { CC_STDCALL, VT_EMPTY, 3,
{ VT_I4,
VT_DISPATCH,
VT_BOOL | VT_BYREF } };
return &kNewProcess;
}

BEGIN_SINK_MAP(Browser)
SINK_ENTRY_INFO(1, DIID_DWebBrowserEvents2, DISPID_BEFORENAVIGATE2, BeforeNavigate2, BeforeNavigate2Info())
SINK_ENTRY_INFO(1, DIID_DWebBrowserEvents2, DISPID_DOCUMENTCOMPLETE, DocumentComplete, DocumentCompleteInfo())
SINK_ENTRY_INFO(1, DIID_DWebBrowserEvents2, DISPID_ONQUIT, OnQuit, NoArgumentsInfo())
SINK_ENTRY_INFO(1, DIID_DWebBrowserEvents2, DISPID_NEWWINDOW3, NewWindow3, NewWindow3Info())
SINK_ENTRY_INFO(1, DIID_DWebBrowserEvents2, DISPID_NEWPROCESS, NewProcess, NewProcessInfo())
END_SINK_MAP()

STDMETHOD_(void, BeforeNavigate2)(IDispatch* pObject, VARIANT* pvarUrl, VARIANT* pvarFlags,
VARIANT* pvarTargetFrame, VARIANT* pvarData, VARIANT* pvarHeaders, VARIANT_BOOL* pbCancel);
STDMETHOD_(void, DocumentComplete)(IDispatch* pDisp, VARIANT* URL);
STDMETHOD_(void, OnQuit)();
STDMETHOD_(void, NewWindow3)(IDispatch** ppDisp, VARIANT_BOOL* pbCancel, DWORD dwFlags, BSTR bstrUrlContext, BSTR bstrUrl);
STDMETHOD_(void, NewProcess)(DWORD lCauseFlag, IDispatch* pWB2, VARIANT_BOOL* pbCancel);

bool Wait(const std::string& page_load_strategy);
void Close(void);
Expand Down Expand Up @@ -115,6 +137,10 @@ class Browser : public DocumentHost, public IDispEventSimpleImpl<1, Browser, &DI
bool IsFullScreen(void);
bool SetFullScreen(bool is_full_screen);

bool IsCrossZoneUrl(std::string url);
void InitiateBrowserReattach(void);
void ReattachBrowser(IWebBrowser2* browser);

bool is_explicit_close_requested(void) const {
return this->is_explicit_close_requested_;
}
Expand All @@ -128,12 +154,15 @@ class Browser : public DocumentHost, public IDispEventSimpleImpl<1, Browser, &DI
bool GetDocumentFromWindow(IHTMLWindow2* window, IHTMLDocument2** doc);
void CheckDialogType(HWND dialog_window_handle);

bool IsProtectedMode(void);

static unsigned int WINAPI GoBackThreadProc(LPVOID param);
static unsigned int WINAPI GoForwardThreadProc(LPVOID param);

CComPtr<IWebBrowser2> browser_;
bool is_navigation_started_;
bool is_explicit_close_requested_;
std::vector<DWORD> known_process_ids_;
};

} // namespace webdriver
Expand Down
31 changes: 25 additions & 6 deletions cpp/iedriver/BrowserFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,16 @@ bool BrowserFactory::AttachToBrowser(ProcessWindowInfo* process_window_info,
return attached;
}

bool BrowserFactory::IsBrowserProcessInitialized(DWORD process_id) {
ProcessWindowInfo info;
info.dwProcessId = process_id;
info.hwndBrowser = NULL;
info.pBrowser = NULL;
::EnumWindows(&BrowserFactory::FindBrowserWindow,
reinterpret_cast<LPARAM>(&info));
return info.hwndBrowser != NULL;
}

bool BrowserFactory::AttachToBrowserUsingActiveAccessibility
(ProcessWindowInfo* process_window_info,
std::string* error_message) {
Expand Down Expand Up @@ -702,7 +712,7 @@ int BrowserFactory::GetZoomLevel(IHTMLDocument2* document, IHTMLWindow2* window)
return zoom;
}

IWebBrowser2* BrowserFactory::CreateBrowser() {
IWebBrowser2* BrowserFactory::CreateBrowser(bool is_protected_mode) {
LOG(TRACE) << "Entering BrowserFactory::CreateBrowser";

IWebBrowser2* browser = NULL;
Expand All @@ -713,11 +723,20 @@ IWebBrowser2* BrowserFactory::CreateBrowser() {
context = context | CLSCTX_ENABLE_CLOAKING;
}

HRESULT hr = ::CoCreateInstance(CLSID_InternetExplorer,
NULL,
context,
IID_IWebBrowser2,
reinterpret_cast<void**>(&browser));
HRESULT hr = S_OK;
if (is_protected_mode) {
hr = ::CoCreateInstance(CLSID_InternetExplorer,
NULL,
context,
IID_IWebBrowser2,
reinterpret_cast<void**>(&browser));
} else {
hr = ::CoCreateInstance(CLSID_InternetExplorerMedium,
NULL,
context,
IID_IWebBrowser2,
reinterpret_cast<void**>(&browser));
}
// When IWebBrowser2::Quit() is called, the wrapper process doesn't
// exit right away. When that happens, CoCreateInstance can fail while
// the abandoned iexplore.exe instance is still valid. The "right" way
Expand Down
3 changes: 2 additions & 1 deletion cpp/iedriver/BrowserFactory.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,12 @@ class BrowserFactory {
void Initialize(BrowserFactorySettings settings);

DWORD LaunchBrowserProcess(std::string* error_message);
IWebBrowser2* CreateBrowser();
IWebBrowser2* CreateBrowser(bool is_protected_mode);
bool AttachToBrowser(ProcessWindowInfo* procWinInfo,
std::string* error_message);
bool GetDocumentFromWindowHandle(HWND window_handle,
IHTMLDocument2** document);
bool IsBrowserProcessInitialized(DWORD process_id);

bool ignore_protected_mode_settings(void) const { return this->ignore_protected_mode_settings_; }
bool ignore_zoom_setting(void) const { return this->ignore_zoom_setting_; }
Expand Down
Loading

0 comments on commit 91ea70d

Please sign in to comment.