Improving recognition of Protected Mode boundary crossing in IE

In particular, when navigating, properly detect URLs beginning with
"about:blank", not just that being the entire URL.

cpp/iedriver/Browser.cpp

@@ -17,7 +17,6 @@
 #include "Browser.h"
 
 #include <comutil.h>
-#include <IEPMapi.h>
 #include <ShlGuid.h>
 
 #include "errorcodes.h"
@@ -86,15 +85,16 @@ void __stdcall Browser::OnQuit() {
                     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.";
+      LOG(WARN) << "This instance of Internet Explorer (" << this->browser_id()
+                << ") 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();
@@ -177,55 +177,6 @@ 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";
@@ -908,21 +859,3 @@ 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

cpp/iedriver/Browser.h

@@ -137,7 +137,6 @@ 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);
 
@@ -154,8 +153,6 @@ 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);
 

cpp/iedriver/DocumentHost.cpp

@@ -16,12 +16,15 @@
 
 #include "DocumentHost.h"
 
+#include <IEPMapi.h>
+
 #include "errorcodes.h"
 #include "logging.h"
 
 #include "BrowserCookie.h"
 #include "BrowserFactory.h"
 #include "CookieManager.h"
+#include "HookProcessor.h"
 #include "messages.h"
 #include "RegistryUtilities.h"
 #include "Script.h"
@@ -385,4 +388,76 @@ bool DocumentHost::GetDocumentDimensions(IHTMLDocument2* doc, LocationInfo* info
   return true;
 }
 
+bool DocumentHost::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.find("about:blank") != 0) {
+    // If the URL starts with "about:blank", it won't cross the Protected
+    // Mode boundary, so skip checking if it's a Protected Mode URL.
+    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 DocumentHost::IsProtectedMode() {
+  LOG(TRACE) << "Entering DocumentHost::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;
+}
+
 } // 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

cpp/iedriver/DocumentHost.h

@@ -64,7 +64,8 @@ class DocumentHost {
   virtual bool SetFullScreen(bool is_full_screen) = 0;
   void Restore(void);
 
-  virtual bool IsCrossZoneUrl(std::string url) = 0;
+  virtual bool IsProtectedMode(void);
+  virtual bool IsCrossZoneUrl(std::string url);
   virtual void InitiateBrowserReattach(void) = 0;
   virtual void ReattachBrowser(IWebBrowser2* browser) = 0;
 

cpp/iedriver/HtmlDialog.h

@@ -77,7 +77,6 @@ class HtmlDialog : public DocumentHost, public IDispEventSimpleImpl<1, HtmlDialo
   bool IsFullScreen(void);
   bool SetFullScreen(bool is_full_screen);
 
-  bool IsCrossZoneUrl(std::string url) { return false; }
   void InitiateBrowserReattach(void) {};
   void ReattachBrowser(IWebBrowser2* browser) {};
 

cpp/iedriver/IECommandExecutor.cpp

@@ -1037,6 +1037,12 @@ bool IECommandExecutor::HandleUnexpectedAlert(BrowserHandle browser,
                                               bool force_use_dismiss,
                                               std::string* alert_text) {
   LOG(TRACE) << "Entering IECommandExecutor::HandleUnexpectedAlert";
+  clock_t end = clock() + 5 * CLOCKS_PER_SEC;
+  bool is_visible = (::IsWindowVisible(alert_handle) == TRUE);
+  while (!is_visible && clock() < end) {
+    ::Sleep(50);
+    is_visible = (::IsWindowVisible(alert_handle) == TRUE);
+  }
   Alert dialog(browser, alert_handle);
   *alert_text = dialog.GetText();
   if (!dialog.is_standard_alert()) {
@@ -1064,6 +1070,7 @@ bool IECommandExecutor::HandleUnexpectedAlert(BrowserHandle browser,
       dialog.Accept();
     }
   }
+
   bool is_notify_unexpected_alert =
     this->unexpected_alert_behavior_.size() == 0 ||
     this->unexpected_alert_behavior_ == IGNORE_UNEXPECTED_ALERTS ||
@@ -1188,6 +1195,14 @@ std::string IECommandExecutor::OpenNewBrowsingContext(const std::string& window_
 std::string IECommandExecutor::OpenNewBrowserWindow(const std::wstring& url) {
   LOG(TRACE) << "Entering IECommandExecutor::OpenNewBrowserWindow";
   bool is_protected_mode_url = ::IEIsProtectedModeURL(url.c_str()) == S_OK;
+  if (url.find(L"about:blank") == 0) {
+    // Special-case URLs starting with about:blank, so that the new window
+    // is in the same Protected Mode zone as the current window from which
+    // it's being opened.
+    BrowserHandle current_browser;
+    this->GetCurrentBrowser(&current_browser);
+    is_protected_mode_url = current_browser->IsProtectedMode();
+  }
   CComPtr<IWebBrowser2> browser = this->factory_->CreateBrowser(is_protected_mode_url);
   if (browser == NULL) {
     // No browser was created, so we have to bail early.
@@ -1219,7 +1234,7 @@ std::string IECommandExecutor::OpenNewBrowserTab(const std::wstring& url) {
 
   std::vector<HWND> original_handles;
   ::EnumChildWindows(top_level_handle,
-                     &FindAllBrowserHandles,
+                     &IECommandExecutor::FindAllBrowserHandles,
                      reinterpret_cast<LPARAM>(&original_handles));
   std::sort(original_handles.begin(), original_handles.end());
 
@@ -1239,7 +1254,7 @@ std::string IECommandExecutor::OpenNewBrowserTab(const std::wstring& url) {
   clock_t end_time = clock() + 5 * CLOCKS_PER_SEC;
   std::vector<HWND> new_handles;
   ::EnumChildWindows(top_level_handle,
-                     &FindAllBrowserHandles,
+                     &IECommandExecutor::FindAllBrowserHandles,
                      reinterpret_cast<LPARAM>(&new_handles));
   while (new_handles.size() <= original_handles.size() &&
          clock() < end_time) {
@@ -1265,10 +1280,29 @@ std::string IECommandExecutor::OpenNewBrowserTab(const std::wstring& url) {
                                                        original_handles.end(),
                                                        diff.begin());
   diff.resize(it - diff.begin());
+  if (diff.size() > 1) {
+    std::string handle_list = "";
+    std::vector<HWND>::const_iterator it = diff.begin();
+    for (; it != diff.end(); ++it) {
+      if (handle_list.size() > 0) {
+        handle_list.append(", ");
+      }
+      handle_list.append(StringUtilities::Format("0x%08x", *it));
+    }
+    LOG(DEBUG) << "Found more than one new window handles! Found "
+               << diff.size() << "windows [" << handle_list << "]";
+  }
   HWND new_tab_window = diff[0];
 
   DWORD process_id;
   ::GetWindowThreadProcessId(new_tab_window, &process_id);
+  clock_t end = clock() + (DEFAULT_BROWSER_REATTACH_TIMEOUT_IN_MILLISECONDS / 1000 * CLOCKS_PER_SEC);
+  bool is_ready = this->factory_->IsBrowserProcessInitialized(process_id);
+  while (!is_ready && clock() < end) {
+    ::Sleep(100);
+    is_ready = this->factory_->IsBrowserProcessInitialized(process_id);
+  }
+
   ProcessWindowInfo info;
   info.dwProcessId = process_id;
   info.hwndBrowser = new_tab_window;
@@ -1278,6 +1312,8 @@ std::string IECommandExecutor::OpenNewBrowserTab(const std::wstring& url) {
   BrowserHandle new_tab_wrapper(new Browser(info.pBrowser,
                                             NULL,
                                             this->m_hWnd));
+  // Force a wait cycle to make sure the browser is finished initializing.
+  new_tab_wrapper->Wait(NORMAL_PAGE_LOAD_STRATEGY);
   this->AddManagedBrowser(new_tab_wrapper);
   return new_tab_wrapper->browser_id();
 }

cpp/iedriver/IEDriver.rc

@@ -50,8 +50,8 @@ END
 //
 
 VS_VERSION_INFO VERSIONINFO
- FILEVERSION 3,141,5,8
- PRODUCTVERSION 3,141,5,8
+ FILEVERSION 3,141,5,9
+ PRODUCTVERSION 3,141,5,9
  FILEFLAGSMASK 0x3fL
 #ifdef _DEBUG
  FILEFLAGS 0x1L
@@ -68,12 +68,12 @@ BEGIN
         BEGIN
             VALUE "CompanyName", "Software Freedom Conservancy"
             VALUE "FileDescription", "Driver library for the IE driver"
-            VALUE "FileVersion", "3.141.5.8"
+            VALUE "FileVersion", "3.141.5.9"
             VALUE "InternalName", "IEDriver.dll"
             VALUE "LegalCopyright", "Copyright (C) 2019"
             VALUE "OriginalFilename", "IEDriver.dll"
             VALUE "ProductName", "Selenium WebDriver"
-            VALUE "ProductVersion", "3.141.5.8"
+            VALUE "ProductVersion", "3.141.5.9"
         END
     END
     BLOCK "VarFileInfo"

cpp/iedriverserver/CHANGELOG

@@ -10,17 +10,25 @@ private releases checked into the prebuilts directory of the source tree, but
 not made generally available on the downloads page.
 
 
+v3.141.5.9
+==========
+ * Enabled serialization of the FileList object from file upload elements
+   (the return value of the files property in JavaScript).
+ * Improved recognition of Protected Mode boundary crossing on navigation,
+   particularly with URLs beginning with "about:blank"
+
 v3.141.5.8
 ==========
- * 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.
+ * Improved driver use with invalid Protected Mode settings. Currently,
+   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