Restrict unsafe URLs in Chrome Frame

Further tighten down what URLs can be loaded in Chrome Frame.
Based on the feedback from the security review and code
inspection, restrict about: scheme only to about:blank
and about:version by default. Factor out logic blocking logic
including for URL zone checking so that all ActiveX, ActiveDoc
and NPAPI will follow the same path. As a result we now block
restricted URL zones in NPAPI instance as well.

Another side effect of this refactoring is that the registry
flag, EnableGcfProtocol, is replaced by AllowUnsafeURLs. If
If this flag is set, then all the security related checking
is turned off.

BUG=50741
TEST=By default gcf: works only for about:blank, about:version and 
view-source of http and https. Setting AllowUnsafeURLs to a non
zero value should allow any URL be loaded via gcf:

Review URL: http://codereview.chromium.org/3159006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@56096 0039d316-1c4b-4281-b951-d872f2087c98

13 files changed, 251 insertions, 226 deletions

diff --git a/chrome_frame/bho.cc b/chrome_frame/bho.cc
index 62ffa8e..f8d44e6 100644
--- a/chrome_frame/bho.cc
+++ b/chrome_frame/bho.cc
@@ -300,7 +300,7 @@ void Bho::ProcessOptInUrls(IWebBrowser2* browser, BSTR url) {
 #endif
 
   std::wstring current_url(url, SysStringLen(url));
-  if (IsValidUrlScheme(current_url, false)) {
+  if (IsValidUrlScheme(GURL(current_url), false)) {
     bool cf_protocol = StartsWith(current_url, kChromeProtocolPrefix, false);
     if (!cf_protocol && IsOptInUrl(current_url.c_str())) {
       DLOG(INFO) << "Opt-in URL. Switching to cf.";
diff --git a/chrome_frame/chrome_active_document.cc b/chrome_frame/chrome_active_document.cc
index 1c03c94..0890c99 100644
--- a/chrome_frame/chrome_active_document.cc
+++ b/chrome_frame/chrome_active_document.cc
@@ -42,8 +42,6 @@
 #include "chrome_frame/crash_reporting/crash_metrics.h"
 #include "chrome_frame/utils.h"
 
-const wchar_t kChromeAttachExternalTabPrefix[] = L"attach_external_tab";
-
 static const wchar_t kUseChromeNetworking[] = L"UseChromeNetworking";
 static const wchar_t kHandleTopLevelRequests[] = L"HandleTopLevelRequests";
 
@@ -124,14 +122,6 @@ HRESULT ChromeActiveDocument::FinalConstruct() {
     LoadAccelerators(this_module,
                      MAKEINTRESOURCE(IDR_CHROME_FRAME_IE_FULL_TAB));
   DCHECK(accelerator_table_ != NULL);
-
-  HRESULT hr = security_manager_.CreateInstance(CLSID_InternetSecurityManager);
-  if (FAILED(hr)) {
-    NOTREACHED() << __FUNCTION__
-                 << " Failed to create InternetSecurityManager. Error: 0x%x"
-                 << hr;
-  }
-
   return S_OK;
 }
 
@@ -273,10 +263,6 @@ STDMETHODIMP ChromeActiveDocument::Load(BOOL fully_avalable,
     return E_INVALIDARG;
   }
 
-  if (!CanNavigateInFullTabMode(cf_url, security_manager_)) {
-    return E_INVALIDARG;
-  }
-
   std::string referrer(mgr ? mgr->referrer() : EmptyString());
 
   // With CTransaction patch we have more robust way to grab the referrer for
@@ -289,12 +275,12 @@ STDMETHODIMP ChromeActiveDocument::Load(BOOL fully_avalable,
   }
 
   if (!LaunchUrl(cf_url, referrer)) {
-    NOTREACHED() << __FUNCTION__ << " Failed to launch url:" << url;
+    DLOG(ERROR) << __FUNCTION__ << " Failed to launch url:" << url;
     return E_INVALIDARG;
   }
 
   if (!cf_url.is_chrome_protocol() && !cf_url.attach_to_external_tab())
-    url_fetcher_->SetInfoForUrl(cf_url.url(), moniker_name, bind_context);
+    url_fetcher_->SetInfoForUrl(url.c_str(), moniker_name, bind_context);
 
   THREAD_SAFE_UMA_HISTOGRAM_CUSTOM_COUNTS("ChromeFrame.FullTabLaunchType",
                                           cf_url.is_chrome_protocol(),
@@ -405,10 +391,6 @@ STDMETHODIMP ChromeActiveDocument::LoadHistory(IStream* stream,
     return E_INVALIDARG;
   }
 
-  if (!CanNavigateInFullTabMode(cf_url, security_manager_)) {
-    return E_INVALIDARG;
-  }
-
   const std::string& referrer = EmptyString();
   if (!LaunchUrl(cf_url, referrer)) {
     NOTREACHED() << __FUNCTION__ << " Failed to launch url:" << url;
@@ -735,10 +717,12 @@ void ChromeActiveDocument::UpdateNavigationState(
   // an external tab container within chrome and then connecting to it from IE.
   // We still want to update the address bar/history, etc, to ensure that
   // the special URL used by Chrome to indicate this is updated correctly.
+  ChromeFrameUrl cf_url;
+  bool is_attach_external_tab_url = cf_url.Parse(std::wstring(url_)) &&
+      cf_url.attach_to_external_tab();
   bool is_internal_navigation = ((new_navigation_info.navigation_index > 0) &&
       (new_navigation_info.navigation_index !=
-       navigation_info_.navigation_index)) ||
-       MatchPatternWide(static_cast<BSTR>(url_), kChromeFrameAttachTabPattern);
+       navigation_info_.navigation_index)) || is_attach_external_tab_url;
 
   if (new_navigation_info.url.is_valid())
     url_.Allocate(UTF8ToWide(new_navigation_info.url.spec()).c_str());
@@ -991,18 +975,14 @@ HRESULT ChromeActiveDocument::IEExec(const GUID* cmd_group_guid,
 bool ChromeActiveDocument::LaunchUrl(const ChromeFrameUrl& cf_url,
                                      const std::string& referrer) {
   DCHECK(automation_client_.get() != NULL);
-  DCHECK(!cf_url.url().empty());
-
-  url_.Allocate(cf_url.url().c_str());
-
-  std::string utf8_url(WideToUTF8(cf_url.url()));
-  DLOG(INFO) << "this:" << this << " url is:" << utf8_url;
+  DCHECK(!cf_url.gurl().is_empty());
 
+  url_.Allocate(UTF8ToWide(cf_url.gurl().spec()).c_str());
   if (cf_url.attach_to_external_tab()) {
     dimensions_ = cf_url.dimensions();
     automation_client_->AttachExternalTab(cf_url.cookie());
     SetWindowDimensions();
-  } else if (!automation_client_->InitiateNavigation(utf8_url,
+  } else if (!automation_client_->InitiateNavigation(cf_url.gurl().spec(),
                                                      referrer,
                                                      is_privileged_)) {
     DLOG(ERROR) << "Invalid URL: " << url_;
@@ -1015,10 +995,8 @@ bool ChromeActiveDocument::LaunchUrl(const ChromeFrameUrl& cf_url,
     return true;
 
   automation_client_->SetUrlFetcher(url_fetcher_.get());
-
-  GURL url(utf8_url);
   return InitializeAutomation(GetHostProcessName(false), L"", IsIEInPrivate(),
-                              false, url, GURL(referrer));
+                              false, cf_url.gurl(), GURL(referrer));
 }
 
 
diff --git a/chrome_frame/chrome_active_document.h b/chrome_frame/chrome_active_document.h
index 7f9fd9d..146f091 100644
--- a/chrome_frame/chrome_active_document.h
+++ b/chrome_frame/chrome_active_document.h
@@ -458,7 +458,6 @@ END_EXEC_COMMAND_MAP()
   // a new ChromeActiveDocument instance is taking its place.
   bool is_automation_client_reused_;
 
-  ScopedComPtr<IInternetSecurityManager> security_manager_;
   ScopedComPtr<INewWindowManager> popup_manager_;
   bool popup_allowed_;
   HACCEL accelerator_table_;
diff --git a/chrome_frame/chrome_frame_activex_base.h b/chrome_frame/chrome_frame_activex_base.h
index a2774e3..9e6734b 100644
--- a/chrome_frame/chrome_frame_activex_base.h
+++ b/chrome_frame/chrome_frame_activex_base.h
@@ -606,9 +606,6 @@ END_MSG_MAP()
     std::string src_utf8;
     WideToUTF8(src, SysStringLen(src), &src_utf8);
     std::string full_url = ResolveURL(GetDocumentUrl(), src_utf8);
-    if (full_url.empty()) {
-      return E_INVALIDARG;
-    }
 
     // We can initiate navigation here even if ready_state is not complete.
     // We do not have to set proxy, and AutomationClient will take care
diff --git a/chrome_frame/chrome_frame_automation.cc b/chrome_frame/chrome_frame_automation.cc
index 216b7a9..05109c5 100644
--- a/chrome_frame/chrome_frame_automation.cc
+++ b/chrome_frame/chrome_frame_automation.cc
@@ -609,6 +609,13 @@ bool ChromeFrameAutomationClient::Initialize(
   // InitializeComplete is called successfully.
   init_state_ = INITIALIZING;
 
+  HRESULT hr = security_manager_.CreateInstance(CLSID_InternetSecurityManager);
+  if (FAILED(hr)) {
+    NOTREACHED() << __FUNCTION__
+                 << " Failed to create InternetSecurityManager. Error: 0x%x"
+                 << hr;
+  }
+
   if (chrome_launch_params_->url().is_valid())
     navigate_after_initialization_ = false;
 
@@ -657,7 +664,7 @@ void ChromeFrameAutomationClient::Uninitialize() {
   if (::IsWindow(m_hWnd))
     DestroyWindow();
 
-  DCHECK(navigate_after_initialization_ == false);
+  // DCHECK(navigate_after_initialization_ == false);
   handle_top_level_requests_ = false;
   ui_thread_id_ = 0;
   chrome_frame_delegate_ = NULL;
@@ -670,11 +677,11 @@ bool ChromeFrameAutomationClient::InitiateNavigation(const std::string& url,
     return false;
 
   GURL parsed_url(url);
+
   // Catch invalid URLs early.
-  if (!parsed_url.is_valid() ||
-      !IsValidUrlScheme(UTF8ToWide(url), is_privileged)) {
-    DLOG(ERROR) << "Invalid URL passed to InitiateNavigation: " << url
-                << " is_privileged=" << is_privileged;
+  // Can we allow this navigation to happen?
+  if (!CanNavigate(parsed_url, security_manager_, is_privileged)) {
+    DLOG(ERROR) << __FUNCTION__ << " Not allowing navigation to: " << url;
     return false;
   }
 
@@ -1357,6 +1364,14 @@ void ChromeFrameAutomationClient::RunUnloadHandlers(HWND notification_window,
   }
 }
 
+void ChromeFrameAutomationClient::SetUrlFetcher(
+    PluginUrlRequestManager* url_fetcher) {
+  DCHECK(url_fetcher != NULL);
+  url_fetcher_ = url_fetcher;
+  url_fetcher_flags_ = url_fetcher->GetThreadSafeFlags();
+  url_fetcher_->set_delegate(this);
+}
+
 void ChromeFrameAutomationClient::SetZoomLevel(PageZoom::Function zoom_level) {
   if (automation_server_) {
     automation_server_->Send(new AutomationMsg_SetZoomLevel(0, tab_handle_,
diff --git a/chrome_frame/chrome_frame_automation.h b/chrome_frame/chrome_frame_automation.h
index 4a3fcc5..e2bdfa0 100644
--- a/chrome_frame/chrome_frame_automation.h
+++ b/chrome_frame/chrome_frame_automation.h
@@ -397,6 +397,9 @@ class ChromeFrameAutomationClient
     handle_top_level_requests_ = handle_top_level_requests;
   }
 
+  // Url request manager set up.
+  void SetUrlFetcher(PluginUrlRequestManager* url_fetcher);
+
   // Called if the same instance of the ChromeFrameAutomationClient object
   // is reused.
   bool Reinitialize(ChromeFrameDelegate* chrome_frame_delegate,
@@ -455,6 +458,20 @@ class ChromeFrameAutomationClient
   // Helpers
   void ReportNavigationError(AutomationMsg_NavigationResponseValues error_code,
                              const std::string& url);
+
+  bool ProcessUrlRequestMessage(TabProxy* tab, const IPC::Message& msg,
+                                bool ui_thread);
+
+  // PluginUrlRequestDelegate implementation. Simply adds tab's handle
+  // as parameter and forwards to Chrome via IPC.
+  virtual void OnResponseStarted(int request_id, const char* mime_type,
+      const char* headers, int size, base::Time last_modified,
+      const std::string& redirect_url, int redirect_status);
+  virtual void OnReadComplete(int request_id, const std::string& data);
+  virtual void OnResponseEnd(int request_id, const URLRequestStatus& status);
+  virtual void OnCookiesRetrieved(bool success, const GURL& url,
+      const std::string& cookie_string, int cookie_id);
+
   bool is_initialized() const {
     return init_state_ == INITIALIZED;
   }
@@ -497,34 +514,16 @@ class ChromeFrameAutomationClient
 
   scoped_refptr<ChromeFrameLaunchParams> chrome_launch_params_;
 
+  // Cache security manager for URL zone checking
+  ScopedComPtr<IInternetSecurityManager> security_manager_;
+
   // When host network stack is used, this object is in charge of
   // handling network requests.
   PluginUrlRequestManager* url_fetcher_;
   PluginUrlRequestManager::ThreadSafeFlags url_fetcher_flags_;
 
-  bool ProcessUrlRequestMessage(TabProxy* tab, const IPC::Message& msg,
-                                bool ui_thread);
-
-  // PluginUrlRequestDelegate implementation. Simply adds tab's handle
-  // as parameter and forwards to Chrome via IPC.
-  virtual void OnResponseStarted(int request_id, const char* mime_type,
-      const char* headers, int size, base::Time last_modified,
-      const std::string& redirect_url, int redirect_status);
-  virtual void OnReadComplete(int request_id, const std::string& data);
-  virtual void OnResponseEnd(int request_id, const URLRequestStatus& status);
-  virtual void OnCookiesRetrieved(bool success, const GURL& url,
-      const std::string& cookie_string, int cookie_id);
-
   friend class BeginNavigateContext;
   friend class CreateExternalTabContext;
-
- public:
-  void SetUrlFetcher(PluginUrlRequestManager* url_fetcher) {
-    DCHECK(url_fetcher != NULL);
-    url_fetcher_ = url_fetcher;
-    url_fetcher_flags_ = url_fetcher->GetThreadSafeFlags();
-    url_fetcher_->set_delegate(this);
-  }
 };
 
 #endif  // CHROME_FRAME_CHROME_FRAME_AUTOMATION_H_
diff --git a/chrome_frame/chrome_frame_npapi.cc b/chrome_frame/chrome_frame_npapi.cc
index 52151a7..e9ee6e9 100644
--- a/chrome_frame/chrome_frame_npapi.cc
+++ b/chrome_frame/chrome_frame_npapi.cc
@@ -1093,17 +1093,25 @@ bool ChromeFrameNPAPI::NavigateToURL(const NPVariant* args, uint32_t arg_count,
   }
 
   std::string url("about:blank");
-
   if (!NPVARIANT_IS_NULL(args[0])) {
     const NPString& str = args[0].value.stringValue;
     if (str.UTF8Length) {
       url.assign(std::string(str.UTF8Characters, str.UTF8Length));
     }
   }
-  DLOG(WARNING) << __FUNCTION__ << " " << url;
+
+  GURL document_url(GetDocumentUrl());
+  if (document_url.SchemeIsSecure()) {
+    GURL source_url(url);
+    if (!source_url.SchemeIsSecure()) {
+      DLOG(WARNING) << __FUNCTION__ << " Prevnting navigation to HTTP url"
+          " since the containing document is HTTPS. URL: " << source_url <<
+          " Document URL: " << document_url;
+      return false;
+    }
+  }
+
   std::string full_url = ResolveURL(GetDocumentUrl(), url);
-  if (full_url.empty())
-    return false;
 
   src_ = full_url;
   // Navigate only if we completed initialization i.e. proxy is set etc.
diff --git a/chrome_frame/protocol_sink_wrap.cc b/chrome_frame/protocol_sink_wrap.cc
index 3388b16..0b3e5a7 100644
--- a/chrome_frame/protocol_sink_wrap.cc
+++ b/chrome_frame/protocol_sink_wrap.cc
@@ -583,7 +583,8 @@ bool HandleAttachToExistingExternalTab(LPCWSTR url,
                                        IInternetProtocol* protocol,
                                        IInternetProtocolSink* prot_sink,
                                        IBindCtx* bind_ctx) {
-  if (MatchPatternWide(url, kChromeFrameAttachTabPattern)) {
+  ChromeFrameUrl cf_url;
+  if (cf_url.Parse(url) && cf_url.attach_to_external_tab()) {
     scoped_refptr<ProtData> prot_data = ProtData::DataFromProtocol(protocol);
     if (!prot_data) {
       // Pass NULL as the read function which indicates that always return EOF
diff --git a/chrome_frame/test/reliability/page_load_test.cc b/chrome_frame/test/reliability/page_load_test.cc
index 6d701cd..ae0fd3e 100644
--- a/chrome_frame/test/reliability/page_load_test.cc
+++ b/chrome_frame/test/reliability/page_load_test.cc
@@ -384,12 +384,12 @@ class PageLoadTest : public testing::Test {
     }
 
     SetConfigBool(kChromeFrameHeadlessMode, true);
-    SetConfigBool(kEnableGCFProtocol, true);
+    SetConfigBool(kAllowUnsafeURLs, true);
   }
 
   virtual void TearDown() {
     DeleteConfigValue(kChromeFrameHeadlessMode);
-    DeleteConfigValue(kEnableGCFProtocol);
+    DeleteConfigValue(kAllowUnsafeURLs);
   }
 
   FilePath ConstructSavedDebugLogPath(const FilePath& debug_log_path,
diff --git a/chrome_frame/test/test_with_web_server.cc b/chrome_frame/test/test_with_web_server.cc
index 2db6172..dead3d2 100644
--- a/chrome_frame/test/test_with_web_server.cc
+++ b/chrome_frame/test/test_with_web_server.cc
@@ -77,7 +77,7 @@ void ChromeFrameTestWithWebServer::SetUp() {
   CloseAllBrowsers();
 
   // Make sure that we are not accidently enabling gcf protocol.
-  SetConfigBool(kEnableGCFProtocol, false);
+  SetConfigBool(kAllowUnsafeURLs, false);
 
   PathService::Get(base::DIR_SOURCE_ROOT, &test_file_path_);
   test_file_path_ = test_file_path_.Append(FILE_PATH_LITERAL("chrome_frame"))
@@ -766,9 +766,9 @@ TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_MetaTag) {
 const wchar_t kCFProtocolPage[] = L"files/cf_protocol.html";
 TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_CFProtocol) {
   // Temporarily enable  gcf: protocol for this test.
-  SetConfigBool(kEnableGCFProtocol, true);
+  SetConfigBool(kAllowUnsafeURLs, true);
   SimpleBrowserTest(IE, kCFProtocolPage, L"chrome_frame_protocol");
-  SetConfigBool(kEnableGCFProtocol, false);
+  SetConfigBool(kAllowUnsafeURLs, false);
 }
 
 const wchar_t kPersistentCookieTest[] =
diff --git a/chrome_frame/test/util_unittests.cc b/chrome_frame/test/util_unittests.cc
index b4da4c4..95c55c7 100644
--- a/chrome_frame/test/util_unittests.cc
+++ b/chrome_frame/test/util_unittests.cc
@@ -93,7 +93,8 @@ TEST(UtilTests, IsValidUrlScheme) {
 
   for (int i = 0; i < arraysize(test_cases); ++i) {
     const Cases& test = test_cases[i];
-    EXPECT_EQ(test.expected, IsValidUrlScheme(test.url, test.is_privileged));
+    EXPECT_EQ(test.expected, IsValidUrlScheme(GURL(test.url),
+                                              test.is_privileged));
   }
 }
 
@@ -118,27 +119,42 @@ TEST(UtilTests, GetTempInternetFiles) {
 TEST(UtilTests, ParseAttachTabUrlTest) {
   ChromeFrameUrl cf_url;
 
-  EXPECT_TRUE(cf_url.Parse(L"http://f/?attach_external_tab&10&1&0&0&100&100"));
+  EXPECT_TRUE(cf_url.Parse(L"http://f/?attach_external_tab&10&1&2&3&123&321"));
 
   EXPECT_TRUE(cf_url.attach_to_external_tab());
   EXPECT_FALSE(cf_url.is_chrome_protocol());
-
   EXPECT_EQ(10, cf_url.cookie());
   EXPECT_EQ(1, cf_url.disposition());
-  EXPECT_EQ(0, cf_url.dimensions().x());
-  EXPECT_EQ(0, cf_url.dimensions().y());
-  EXPECT_EQ(100, cf_url.dimensions().width());
-  EXPECT_EQ(100, cf_url.dimensions().height());
+  EXPECT_EQ(gfx::Rect(2, 3, 123, 321), cf_url.dimensions());
 
-  EXPECT_TRUE(cf_url.Parse(L"http://www.foobar.com?&10&1&0&0&100&100"));
+  EXPECT_TRUE(cf_url.Parse(L"http://www.foobar.com?&10&1&2&3&123&321"));
   EXPECT_FALSE(cf_url.attach_to_external_tab());
+  EXPECT_FALSE(cf_url.is_chrome_protocol());
+  EXPECT_EQ(0, cf_url.cookie());
+  EXPECT_EQ(0, cf_url.disposition());
+  EXPECT_EQ(gfx::Rect(0, 0, 0, 0), cf_url.dimensions());
 
   EXPECT_FALSE(cf_url.Parse(L"attach_external_tab&10&1"));
-  EXPECT_TRUE(cf_url.attach_to_external_tab());
+  EXPECT_FALSE(cf_url.attach_to_external_tab());
+  EXPECT_FALSE(cf_url.is_chrome_protocol());
+  EXPECT_EQ(0, cf_url.cookie());
+  EXPECT_EQ(0, cf_url.disposition());
+  EXPECT_EQ(gfx::Rect(0, 0, 0, 0), cf_url.dimensions());
+
+  EXPECT_TRUE(cf_url.Parse(L"gcf:http://f/?attach_tab&10&1&2&3&123&321"));
+  EXPECT_FALSE(cf_url.attach_to_external_tab());
+  EXPECT_TRUE(cf_url.is_chrome_protocol());
+  EXPECT_EQ(0, cf_url.cookie());
+  EXPECT_EQ(0, cf_url.disposition());
+  EXPECT_EQ(gfx::Rect(0, 0, 0, 0), cf_url.dimensions());
 
-  EXPECT_TRUE(cf_url.Parse(L"gcf:http://f/?attach_tab&10&1&0&0&100&100"));
+  EXPECT_TRUE(cf_url.Parse(L"gcf:http://google.com"));
   EXPECT_FALSE(cf_url.attach_to_external_tab());
   EXPECT_TRUE(cf_url.is_chrome_protocol());
+  EXPECT_EQ(0, cf_url.cookie());
+  EXPECT_EQ(0, cf_url.disposition());
+  EXPECT_EQ(gfx::Rect(0, 0, 0, 0), cf_url.dimensions());
+  EXPECT_EQ(cf_url.gurl(), GURL("http://google.com"));
 }
 
 // Mock for the IInternetSecurityManager interface
@@ -172,74 +188,91 @@ class MockIInternetSecurityManager : public IInternetSecurityManager {
       HRESULT(DWORD zone, IEnumString** enum_string, DWORD flags));
 };
 
-TEST(UtilTests, CanNavigateFullTabModeTest) {
-  ChromeFrameUrl cf_url;
-
+TEST(UtilTests, CanNavigateTest) {
   MockIInternetSecurityManager mock;
-  EXPECT_CALL(mock, MapUrlToZone(testing::StartsWith(L"http://blah"),
-                                                     testing::_, testing::_))
-    .WillRepeatedly(testing::DoAll(
-        testing::SetArgumentPointee<1>(URLZONE_INTERNET),
-        testing::Return(S_OK)));
-
-  EXPECT_CALL(mock, MapUrlToZone(testing::StartsWith(L"http://untrusted"),
-                                                     testing::_, testing::_))
-    .WillRepeatedly(testing::DoAll(
-        testing::SetArgumentPointee<1>(URLZONE_UNTRUSTED),
-        testing::Return(S_OK)));
-
-  EXPECT_CALL(mock, MapUrlToZone(testing::StartsWith(L"about:"),
-                                                     testing::_, testing::_))
-    .WillRepeatedly(testing::DoAll(
-        testing::SetArgumentPointee<1>(URLZONE_TRUSTED),
-        testing::Return(S_OK)));
-
-  EXPECT_CALL(mock, MapUrlToZone(testing::StartsWith(L"view-source:"),
-                                                     testing::_, testing::_))
-    .WillRepeatedly(testing::DoAll(
-        testing::SetArgumentPointee<1>(URLZONE_TRUSTED),
-        testing::Return(S_OK)));
-
-  EXPECT_TRUE(cf_url.Parse(
-      L"http://blah/?attach_external_tab&10&1&0&0&100&100"));
-  EXPECT_TRUE(CanNavigateInFullTabMode(cf_url, &mock));
-
-  EXPECT_TRUE(cf_url.Parse(
-      L"http://untrusted/bar.html"));
-  EXPECT_FALSE(CanNavigateInFullTabMode(cf_url, &mock));
-
-  EXPECT_TRUE(cf_url.Parse(
-      L"http://blah/?attach_external_tab&10&1&0&0&100&100"));
-  EXPECT_TRUE(CanNavigateInFullTabMode(cf_url, &mock));
 
-  EXPECT_TRUE(cf_url.Parse(L"gcf:about:blank"));
-  EXPECT_TRUE(CanNavigateInFullTabMode(cf_url, &mock));
-
-  EXPECT_TRUE(cf_url.Parse(L"gcf:view-source:http://www.foo.com"));
-  EXPECT_TRUE(CanNavigateInFullTabMode(cf_url, &mock));
-
-  EXPECT_TRUE(cf_url.Parse(L"gcf:about:version"));
-  EXPECT_TRUE(CanNavigateInFullTabMode(cf_url, &mock));
-
-  EXPECT_TRUE(cf_url.Parse(L"gcf:about:bar"));
-  EXPECT_TRUE(CanNavigateInFullTabMode(cf_url, &mock));
-
-  bool enable_gcf = GetConfigBool(false, kEnableGCFProtocol);
+  struct Zones {
+    const wchar_t* url_prefix;
+    URLZONE zone;
+  } test_zones[] = {
+    { L"http://blah", URLZONE_INTERNET },
+    { L"http://untrusted", URLZONE_UNTRUSTED },
+    { L"about:", URLZONE_TRUSTED },
+    { L"view-source:", URLZONE_TRUSTED },
+    { L"chrome-extension:", URLZONE_TRUSTED },
+    { L"ftp:", URLZONE_UNTRUSTED },
+    { L"file:", URLZONE_LOCAL_MACHINE },
+    { L"sip:", URLZONE_UNTRUSTED },
+  };
 
-  SetConfigBool(kEnableGCFProtocol, false);
+  for (int i = 0; i < arraysize(test_zones); ++i) {
+    const Zones& zone = test_zones[i];
+    EXPECT_CALL(mock, MapUrlToZone(testing::StartsWith(zone.url_prefix),
+                                                       testing::_, testing::_))
+      .WillRepeatedly(testing::DoAll(
+          testing::SetArgumentPointee<1>(zone.zone),
+          testing::Return(S_OK)));
+  }
 
-  EXPECT_TRUE(cf_url.Parse(L"gcf:http://blah"));
-  EXPECT_FALSE(CanNavigateInFullTabMode(cf_url, &mock));
+  struct Cases {
+    const char* url;
+    bool is_privileged;
+    bool default_expected;
+    bool unsafe_expected;
+  } test_cases[] = {
+    // Invalid URL
+    { "          ", false, false, false },
+    { "foo bar", true, false, false },
+
+    //non-privileged test cases
+    { "http://blah/?attach_external_tab&10&1&0&0&100&100", false, true, true },
+    { "http://untrusted/bar.html", false, false, true },
+    { "http://blah/?attach_external_tab&10&1&0&0&100&100", false, true, true },
+    { "view-source:http://www.google.ca", false, true, true },
+    { "view-source:javascript:alert('foo');", false, false, true },
+    { "about:blank", false, true, true },
+    { "About:Version", false, true, true },
+    { "about:config", false, false, true },
+    { "chrome-extension://aaaaaaaaaaaaaaaaaaa/toolstrip.html", false, false,
+         true },
+    { "ftp://www.google.ca", false, false, true },
+    { "file://www.google.ca", false, false, true },
+    { "file://C:\boot.ini", false, false, true },
+    { "SIP:someone@10.1.2.3", false, false, true },
+
+    //privileged test cases
+    { "http://blah/?attach_external_tab&10&1&0&0&100&100", true, true, true },
+    { "http://untrusted/bar.html", true, false, true },
+    { "http://blah/?attach_external_tab&10&1&0&0&100&100", true, true, true },
+    { "view-source:http://www.google.ca", true, true, true },
+    { "view-source:javascript:alert('foo');", true, false, true },
+    { "about:blank", true, true, true },
+    { "About:Version", true, true, true },
+    { "about:config", true, false, true },
+    { "chrome-extension://aaaaaaaaaaaaaaaaaaa/toolstrip.html", true, true,
+       true },
+    { "ftp://www.google.ca", true, false, true },
+    { "file://www.google.ca", true, false, true },
+    { "file://C:\boot.ini", true, false, true },
+    { "sip:someone@10.1.2.3", false, false, true },
+  };
 
-  SetConfigBool(kEnableGCFProtocol, true);
+  for (int i = 0; i < arraysize(test_cases); ++i) {
+    const Cases& test = test_cases[i];
+    bool actual = CanNavigate(GURL(test.url), &mock, test.is_privileged);
+    EXPECT_EQ(test.default_expected, actual) << "Failure url: " << test.url;
+  }
 
-  EXPECT_TRUE(cf_url.Parse(L"gcf:http://blah"));
-  EXPECT_TRUE(CanNavigateInFullTabMode(cf_url, &mock));
+  bool enable_gcf = GetConfigBool(false, kAllowUnsafeURLs);
+  SetConfigBool(kAllowUnsafeURLs, true);
 
-  EXPECT_TRUE(cf_url.Parse(L"gcf:http://untrusted/bar"));
-  EXPECT_FALSE(CanNavigateInFullTabMode(cf_url, &mock));
+  for (int i = 0; i < arraysize(test_cases); ++i) {
+    const Cases& test = test_cases[i];
+    bool actual = CanNavigate(GURL(test.url), &mock, test.is_privileged);
+    EXPECT_EQ(test.unsafe_expected, actual) << "Failure url: " << test.url;
+  }
 
-  SetConfigBool(kEnableGCFProtocol, enable_gcf);
+  SetConfigBool(kAllowUnsafeURLs, enable_gcf);
 }
 
 TEST(UtilTests, ParseVersionTest) {
diff --git a/chrome_frame/utils.cc b/chrome_frame/utils.cc
index f60f6c0..c2eade6 100644
--- a/chrome_frame/utils.cc
+++ b/chrome_frame/utils.cc
@@ -41,16 +41,16 @@ const wchar_t kContentAttribName[] = L"content";
 const wchar_t kXUACompatValue[] = L"x-ua-compatible";
 const wchar_t kBodyTag[] = L"body";
 const wchar_t kChromeContentPrefix[] = L"chrome=";
+const char kGCFProtocol[] = "gcf";
 const wchar_t kChromeProtocolPrefix[] = L"gcf:";
 const wchar_t kChromeMimeType[] = L"application/chromepage";
 const wchar_t kPatchProtocols[] = L"PatchProtocols";
-const wchar_t kChromeFrameAttachTabPattern[] = L"*?attach_external_tab&*";
 
 static const wchar_t kChromeFrameConfigKey[] =
     L"Software\\Google\\ChromeFrame";
 static const wchar_t kRenderInGCFUrlList[] = L"RenderInGcfUrls";
 static const wchar_t kRenderInHostUrlList[] = L"RenderInHostUrls";
-static const wchar_t kEnableGCFProtocol[] = L"EnableGCFProtocol";
+static const wchar_t kAllowUnsafeURLs[] = L"AllowUnsafeURLs";
 static const wchar_t kEnableGCFRendererByDefault[] = L"IsDefaultRenderer";
 static const wchar_t kEnableBuggyBhoIntercept[] = L"EnableBuggyBhoIntercept";
 
@@ -62,7 +62,7 @@ static const wchar_t kChromeFramePersistNPAPIReg[] = L"PersistNPAPIReg";
 const wchar_t kChromeFrameOmahaSuffix[] = L"-cf";
 const wchar_t kDevChannelName[] = L"-dev";
 
-const wchar_t kChromeAttachExternalTabPrefix[] = L"?attach_external_tab";
+const char kAttachExternalTabPrefix[] = "attach_external_tab";
 
 // Indicates that we are running in a test environment, where execptions, etc
 // are handled by the chrome test crash server.
@@ -856,21 +856,19 @@ bool CheckForCFNavigation(IBrowserService* browser, bool clear_flag) {
   return ret;
 }
 
-bool IsValidUrlScheme(const std::wstring& url, bool is_privileged) {
-  if (url.empty())
+bool IsValidUrlScheme(const GURL& url, bool is_privileged) {
+  if (url.is_empty())
     return false;
 
-  GURL crack_url(url);
-
-  if (crack_url.SchemeIs(chrome::kHttpScheme) ||
-      crack_url.SchemeIs(chrome::kHttpsScheme) ||
-      crack_url.SchemeIs(chrome::kAboutScheme))
+  if (url.SchemeIs(chrome::kHttpScheme) ||
+      url.SchemeIs(chrome::kHttpsScheme) ||
+      url.SchemeIs(chrome::kAboutScheme))
     return true;
 
   // Additional checking for view-source. Allow only http and https
   // URLs in view source.
-  if (crack_url.SchemeIs(chrome::kViewSourceScheme)) {
-    GURL sub_url(crack_url.path());
+  if (url.SchemeIs(chrome::kViewSourceScheme)) {
+    GURL sub_url(url.path());
     if (sub_url.SchemeIs(chrome::kHttpScheme) ||
         sub_url.SchemeIs(chrome::kHttpsScheme))
       return true;
@@ -879,8 +877,8 @@ bool IsValidUrlScheme(const std::wstring& url, bool is_privileged) {
   }
 
   if (is_privileged &&
-      (crack_url.SchemeIs(chrome::kDataScheme) ||
-       crack_url.SchemeIs(chrome::kExtensionScheme)))
+      (url.SchemeIs(chrome::kDataScheme) ||
+       url.SchemeIs(chrome::kExtensionScheme)))
     return true;
 
   return false;
@@ -1270,49 +1268,34 @@ HRESULT ReadStream(IStream* stream, size_t size, std::string* data) {
   return hr;
 }
 
-ChromeFrameUrl::ChromeFrameUrl()
-    : is_chrome_protocol_(false),
-      attach_to_external_tab_(false),
-      cookie_(0),
-      disposition_(0) {
+ChromeFrameUrl::ChromeFrameUrl() {
+  Reset();
 }
 
 bool ChromeFrameUrl::Parse(const std::wstring& url) {
-  bool ret = false;
-  if (url.empty())
-    return ret;
+  Reset();
+  parsed_url_ = GURL(url);
 
-  url_ = url;
+  if (parsed_url_.is_empty())
+    return false;
 
-  attach_to_external_tab_ = MatchPatternWide(url.c_str(),
-                                             kChromeFrameAttachTabPattern);
-  is_chrome_protocol_ = StartsWith(url, kChromeProtocolPrefix, false);
-  DCHECK(!(attach_to_external_tab_ && is_chrome_protocol_));
+  is_chrome_protocol_ = parsed_url_.SchemeIs(kGCFProtocol);
   if (is_chrome_protocol_) {
-    url_.erase(0, lstrlen(kChromeProtocolPrefix));
+    parsed_url_ = GURL(url.c_str() + lstrlen(kChromeProtocolPrefix));
+    return true;
   }
 
-  if (attach_to_external_tab_) {
-    ret = ParseAttachExternalTabUrl();
-  } else {
-    ret = true;
-  }
-  return ret;
+  return ParseAttachExternalTabUrl();
 }
 
 bool ChromeFrameUrl::ParseAttachExternalTabUrl() {
-  size_t attach_external_tab_start_pos =
-      url_.find(kChromeAttachExternalTabPrefix);
-  if (attach_external_tab_start_pos == std::wstring::npos) {
-    DLOG(ERROR) << "Invalid url:" << url_;
-    return false;
+  std::string query = parsed_url_.query();
+  if (!StartsWithASCII(query, kAttachExternalTabPrefix, false)) {
+    return parsed_url_.is_valid();
   }
 
-  std::wstring url =
-      url_.substr(attach_external_tab_start_pos,
-                  url_.length() - attach_external_tab_start_pos);
-
-  WStringTokenizer tokenizer(url, L"&");
+  attach_to_external_tab_ = true;
+  StringTokenizer tokenizer(query, "&");
   // Skip over kChromeAttachExternalTabPrefix
   tokenizer.GetNext();
   // Read the following items in order.
@@ -1323,79 +1306,86 @@ bool ChromeFrameUrl::ParseAttachExternalTabUrl() {
   // 5. dimension.width
   // 6. dimension.height.
   if (tokenizer.GetNext()) {
-    wchar_t* end_ptr = 0;
-    cookie_ = _wcstoui64(tokenizer.token().c_str(), &end_ptr, 10);
+    char* end_ptr = 0;
+    cookie_ = _strtoui64(tokenizer.token().c_str(), &end_ptr, 10);
   } else {
     return false;
   }
 
   if (tokenizer.GetNext()) {
-    disposition_ = _wtoi(tokenizer.token().c_str());
+    disposition_ = atoi(tokenizer.token().c_str());
   } else {
     return false;
   }
 
   if (tokenizer.GetNext()) {
-    dimensions_.set_x(_wtoi(tokenizer.token().c_str()));
+    dimensions_.set_x(atoi(tokenizer.token().c_str()));
   } else {
     return false;
   }
 
   if (tokenizer.GetNext()) {
-    dimensions_.set_y(_wtoi(tokenizer.token().c_str()));
+    dimensions_.set_y(atoi(tokenizer.token().c_str()));
   } else {
     return false;
   }
 
   if (tokenizer.GetNext()) {
-    dimensions_.set_width(_wtoi(tokenizer.token().c_str()));
+    dimensions_.set_width(atoi(tokenizer.token().c_str()));
   } else {
     return false;
   }
 
   if (tokenizer.GetNext()) {
-    dimensions_.set_height(_wtoi(tokenizer.token().c_str()));
+    dimensions_.set_height(atoi(tokenizer.token().c_str()));
   } else {
     return false;
   }
   return true;
 }
 
-bool CanNavigateInFullTabMode(const ChromeFrameUrl& cf_url,
-                              IInternetSecurityManager* security_manager) {
-  bool is_privileged = false;
+void ChromeFrameUrl::Reset() {
+  attach_to_external_tab_ = false;
+  is_chrome_protocol_ = false;
+  cookie_ = 0;
+  dimensions_.SetRect(0, 0, 0, 0);
+  disposition_ = 0;
+}
 
-  if (!IsValidUrlScheme(cf_url.url(), is_privileged)) {
-    DLOG(WARNING) << __FUNCTION__ << " Disallowing navigation to url: "
-                  << cf_url.url();
+bool CanNavigate(const GURL& url, IInternetSecurityManager* security_manager,
+                 bool is_privileged) {
+  if (!url.is_valid()) {
+    DLOG(ERROR) << "Invalid URL passed to InitiateNavigation: " << url;
     return false;
   }
 
-  if (security_manager) {
-    DWORD zone = URLZONE_INVALID;
-    security_manager->MapUrlToZone(cf_url.url().c_str(), &zone, 0);
-    if (zone == URLZONE_UNTRUSTED) {
+  // No sanity checks if unsafe URLs are allowed
+  if (GetConfigBool(false, kAllowUnsafeURLs))
+    return true;
+
+  if (!IsValidUrlScheme(url, is_privileged)) {
+    DLOG(WARNING) << __FUNCTION__ << " Disallowing navigation to url: " << url;
+    return false;
+  }
+
+  // Allow only about:blank or about:version
+  if (url.SchemeIs(chrome::kAboutScheme)) {
+    if (!LowerCaseEqualsASCII(url.spec(), chrome::kAboutBlankURL) &&
+        !LowerCaseEqualsASCII(url.spec(), chrome::kAboutVersionURL)) {
       DLOG(WARNING) << __FUNCTION__
-                    << " Disallowing navigation to restricted url: "
-                    << cf_url.url();
+                    << " Disallowing navigation to about url: " << url;
       return false;
     }
   }
 
-  if (cf_url.is_chrome_protocol()) {
-    // Allow chrome protocol (gcf:) if -
-    // - explicitly enabled using registry
-    // - for gcf:attach_external_tab
-    // - for gcf:about and gcf:view-source
-    GURL crack_url(cf_url.url());
-    bool allow_gcf_protocol =
-        GetConfigBool(false, kEnableGCFProtocol) ||
-        crack_url.SchemeIs(chrome::kAboutScheme) ||
-        crack_url.SchemeIs(chrome::kViewSourceScheme);
-    if (!allow_gcf_protocol) {
+  // Prevent navigations to URLs in untrusted zone, even in Firefox.
+  if (security_manager) {
+    DWORD zone = URLZONE_INVALID;
+    std::wstring unicode_url = UTF8ToWide(url.spec());
+    security_manager->MapUrlToZone(unicode_url.c_str(), &zone, 0);
+    if (zone == URLZONE_UNTRUSTED) {
       DLOG(WARNING) << __FUNCTION__
-                    << " Disallowing navigation to gcf url: "
-                    << cf_url.url();
+                    << " Disallowing navigation to restricted url: " << url;
       return false;
     }
   }
diff --git a/chrome_frame/utils.h b/chrome_frame/utils.h
index beb7fa1..8640f11 100644
--- a/chrome_frame/utils.h
+++ b/chrome_frame/utils.h
@@ -24,10 +24,11 @@
 // utils.h : Various utility functions and classes
 
 extern const wchar_t kChromeContentPrefix[];
+extern const char kGCFProtocol[];
 extern const wchar_t kChromeProtocolPrefix[];
 extern const wchar_t kChromeFrameHeadlessMode[];
 extern const wchar_t kChromeFrameUnpinnedMode[];
-extern const wchar_t kEnableGCFProtocol[];
+extern const wchar_t kAllowUnsafeURLs[];
 extern const wchar_t kEnableBuggyBhoIntercept[];
 extern const wchar_t kChromeMimeType[];
 extern const wchar_t kChromeFrameAttachTabPattern[];
@@ -276,7 +277,7 @@ bool CheckForCFNavigation(IBrowserService* browser, bool clear_flag);
 // Returns true if the URL passed in is something which can be handled by
 // Chrome. If this function returns false then we should fail the navigation.
 // When is_privileged is true, chrome extension URLs will be considered valid.
-bool IsValidUrlScheme(const std::wstring& url, bool is_privileged);
+bool IsValidUrlScheme(const GURL& url, bool is_privileged);
 
 // Returns the raw http headers for the current request given an
 // IWinInetHttpInfo pointer.
@@ -503,8 +504,8 @@ class ChromeFrameUrl {
     return dimensions_;
   }
 
-  const std::wstring& url() const {
-    return url_;
+  const GURL& gurl() const {
+    return parsed_url_;
   }
 
  private:
@@ -512,19 +513,23 @@ class ChromeFrameUrl {
   // suffix portion of the URL which contains the attach_external_tab prefix.
   bool ParseAttachExternalTabUrl();
 
+  // Clear state.
+  void Reset();
+
   bool attach_to_external_tab_;
   bool is_chrome_protocol_;
-  std::wstring url_;
   uint64 cookie_;
   gfx::Rect dimensions_;
   int disposition_;
+
+  GURL parsed_url_;
 };
 
 // Returns true if we can navigate to this URL.
 // This function checks if the url scheme is valid for navigation within
 // chrome and whether it is a restricted URL as per IE settings. In either of
 // these cases it returns false.
-bool CanNavigateInFullTabMode(const ChromeFrameUrl& cf_url,
-                              IInternetSecurityManager* security_manager);
+bool CanNavigate(const GURL& url, IInternetSecurityManager* security_manager,
+                 bool is_privileged);
 
 #endif  // CHROME_FRAME_UTILS_H_