diff options
author | ananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-12-14 04:34:43 +0000 |
---|---|---|
committer | ananta@chromium.org <ananta@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-12-14 04:34:43 +0000 |
commit | 354bcbaf8603cb5800737f86a84ee465166b2b4e (patch) | |
tree | 863753d52b8f9dfb54e676bbf3d1b38a968df061 /chrome_frame | |
parent | 0e097f90d5bba038b5826ebac2870aadde28d2ca (diff) | |
download | chromium_src-354bcbaf8603cb5800737f86a84ee465166b2b4e.zip chromium_src-354bcbaf8603cb5800737f86a84ee465166b2b4e.tar.gz chromium_src-354bcbaf8603cb5800737f86a84ee465166b2b4e.tar.bz2 |
Add support for gcf:about:plugins in chrome frame full tab mode. The URL validation code path
in ChromeFrame now takes in an interface NavigationConstraints which allows the delegateslike
the ActiveX, ActiveDocument and the NPAPI plugins to control the navigation decisions.
We no longer refer to the InternetSecurityManager interface which is IE only for performing
zone decisions in the ChromeFrame NPAPI plugin.
Fixes bug http://code.google.com/p/chromium/issues/detail?id=66118
BUG=66118
TEST=Covered by additional CanNavigate unit tests.
Review URL: http://codereview.chromium.org/5698005
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@69101 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome_frame')
-rw-r--r-- | chrome_frame/chrome_active_document.cc | 14 | ||||
-rw-r--r-- | chrome_frame/chrome_active_document.h | 3 | ||||
-rw-r--r-- | chrome_frame/chrome_frame.gyp | 3 | ||||
-rw-r--r-- | chrome_frame/chrome_frame_activex_base.h | 24 | ||||
-rw-r--r-- | chrome_frame/chrome_frame_automation.cc | 19 | ||||
-rw-r--r-- | chrome_frame/chrome_frame_automation.h | 9 | ||||
-rw-r--r-- | chrome_frame/chrome_frame_npapi.cc | 4 | ||||
-rw-r--r-- | chrome_frame/chrome_frame_npapi.h | 4 | ||||
-rw-r--r-- | chrome_frame/external_tab.cc | 6 | ||||
-rw-r--r-- | chrome_frame/external_tab.h | 6 | ||||
-rw-r--r-- | chrome_frame/external_tab_test.cc | 8 | ||||
-rw-r--r-- | chrome_frame/navigation_constraints.cc | 69 | ||||
-rw-r--r-- | chrome_frame/navigation_constraints.h | 37 | ||||
-rw-r--r-- | chrome_frame/test/automation_client_mock.cc | 19 | ||||
-rw-r--r-- | chrome_frame/test/util_unittests.cc | 196 | ||||
-rw-r--r-- | chrome_frame/utils.cc | 39 | ||||
-rw-r--r-- | chrome_frame/utils.h | 10 |
17 files changed, 350 insertions, 120 deletions
diff --git a/chrome_frame/chrome_active_document.cc b/chrome_frame/chrome_active_document.cc index deee511..ba2390b 100644 --- a/chrome_frame/chrome_active_document.cc +++ b/chrome_frame/chrome_active_document.cc @@ -501,6 +501,18 @@ STDMETHODIMP ChromeActiveDocument::Next(BSTR* url, BSTR* policy, return S_OK; } +bool ChromeActiveDocument::IsSchemeAllowed(const GURL& url) { + bool allowed = BaseActiveX::IsSchemeAllowed(url); + if (allowed) + return true; + + if (url.SchemeIs(chrome::kAboutScheme)) { + if (LowerCaseEqualsASCII(url.spec(), chrome::kAboutPluginsURL)) + return true; + } + return false; +} + HRESULT ChromeActiveDocument::GetInPlaceFrame( IOleInPlaceFrame** in_place_frame) { DCHECK(in_place_frame); @@ -1032,7 +1044,7 @@ bool ChromeActiveDocument::LaunchUrl(const ChromeFrameUrl& cf_url, SetWindowDimensions(); } else if (!automation_client_->InitiateNavigation(cf_url.gurl().spec(), referrer, - is_privileged_)) { + this)) { DLOG(ERROR) << "Invalid URL: " << url_; Error(L"Invalid URL"); url_.Reset(); diff --git a/chrome_frame/chrome_active_document.h b/chrome_frame/chrome_active_document.h index 5723e7ff..55ee24f 100644 --- a/chrome_frame/chrome_active_document.h +++ b/chrome_frame/chrome_active_document.h @@ -356,6 +356,9 @@ END_EXEC_COMMAND_MAP() STDMETHOD(GetPrivacyImpacted)(BOOL* privacy_impacted); STDMETHOD(Next)(BSTR* url, BSTR* policy, LONG* reserved, DWORD* flags); + // NavigationConstraints overrides. + bool IsSchemeAllowed(const GURL& url); + // Accessor for InPlaceMenu. Returns S_OK if set, S_FALSE if NULL. HRESULT GetInPlaceFrame(IOleInPlaceFrame** in_place_frame); diff --git a/chrome_frame/chrome_frame.gyp b/chrome_frame/chrome_frame.gyp index b04d965..f31ba89 100644 --- a/chrome_frame/chrome_frame.gyp +++ b/chrome_frame/chrome_frame.gyp @@ -420,6 +420,7 @@ 'msvs_settings': { 'VCLinkerTool': { 'LinkIncremental': '<(msvs_large_module_debug_link_mode)', + 'DelayLoadDLLs': ['nspr4.dll'], }, }, }, @@ -929,6 +930,8 @@ 'custom_sync_call_context.h', 'external_tab.h', 'external_tab.cc', + 'navigation_constraints.h', + 'navigation_constraints.cc', 'plugin_url_request.h', 'plugin_url_request.cc', 'sync_msg_reply_dispatcher.h', diff --git a/chrome_frame/chrome_frame_activex_base.h b/chrome_frame/chrome_frame_activex_base.h index 811c314..7d39a7e5 100644 --- a/chrome_frame/chrome_frame_activex_base.h +++ b/chrome_frame/chrome_frame_activex_base.h @@ -29,6 +29,7 @@ #include "chrome_frame/chrome_frame_plugin.h" #include "chrome_frame/com_message_event.h" #include "chrome_frame/com_type_info_holder.h" +#include "chrome_frame/navigation_constraints.h" #include "chrome_frame/simple_resource_loader.h" #include "chrome_frame/urlmon_url_request.h" #include "chrome_frame/urlmon_url_request_private.h" @@ -149,10 +150,6 @@ class ATL_NO_VTABLE ProxyDIChromeFrameEvents extern bool g_first_launch_by_process_; -// Posted when the worker thread used for handling URL requests in IE finishes -// uninitialization. -#define WM_WORKER_THREAD_UNINITIALIZED_MSG (WM_APP + 1) - // Common implementation for ActiveX and Active Document template <class T, const CLSID& class_id> class ATL_NO_VTABLE ChromeFrameActivexBase : // NOLINT @@ -173,7 +170,8 @@ class ATL_NO_VTABLE ChromeFrameActivexBase : // NOLINT public IPropertyNotifySinkCP<T>, public CComCoClass<T, &class_id>, public CComControl<T>, - public ChromeFramePlugin<T> { + public ChromeFramePlugin<T>, + public NavigationConstraintsImpl { protected: typedef std::set<base::win::ScopedComPtr<IDispatch> > EventHandlers; typedef ChromeFrameActivexBase<T, class_id> BasePlugin; @@ -575,6 +573,20 @@ END_MSG_MAP() Fire_onclose(); } + // NavigationConstraints overrides. + virtual bool IsSchemeAllowed(const GURL& url) { + bool allowed = NavigationConstraintsImpl::IsSchemeAllowed(url); + if (allowed) + return true; + + if (is_privileged_ && + (url.SchemeIs(chrome::kDataScheme) || + url.SchemeIs(chrome::kExtensionScheme))) { + return true; + } + return false; + } + // Overridden to take advantage of readystate prop changes and send those // to potential listeners. HRESULT FireOnChanged(DISPID dispid) { @@ -611,7 +623,7 @@ END_MSG_MAP() // of navigation just after CreateExternalTab is done. if (!automation_client_->InitiateNavigation(full_url, GetDocumentUrl(), - is_privileged_)) { + this)) { // TODO(robertshield): Make InitiateNavigation return more useful // error information. return E_INVALIDARG; diff --git a/chrome_frame/chrome_frame_automation.cc b/chrome_frame/chrome_frame_automation.cc index c8a0938..0d7a4b9 100644 --- a/chrome_frame/chrome_frame_automation.cc +++ b/chrome_frame/chrome_frame_automation.cc @@ -27,6 +27,7 @@ #include "chrome_frame/chrome_launcher_utils.h" #include "chrome_frame/crash_reporting/crash_metrics.h" #include "chrome_frame/custom_sync_call_context.h" +#include "chrome_frame/navigation_constraints.h" #include "chrome_frame/simple_resource_loader.h" #include "chrome_frame/utils.h" @@ -649,16 +650,6 @@ bool ChromeFrameAutomationClient::Initialize( init_state_ = INITIALIZING; HRESULT hr = S_OK; - // If chrome crashed and is being restarted, the security_manager_ object - // might already be valid. - if (security_manager_.get() == NULL) - 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; @@ -715,8 +706,10 @@ void ChromeFrameAutomationClient::Uninitialize() { init_state_ = UNINITIALIZED; } -bool ChromeFrameAutomationClient::InitiateNavigation(const std::string& url, - const std::string& referrer, bool is_privileged) { +bool ChromeFrameAutomationClient::InitiateNavigation( + const std::string& url, + const std::string& referrer, + NavigationConstraints* navigation_constraints) { if (url.empty()) return false; @@ -724,7 +717,7 @@ bool ChromeFrameAutomationClient::InitiateNavigation(const std::string& url, // Catch invalid URLs early. // Can we allow this navigation to happen? - if (!CanNavigate(parsed_url, security_manager_, is_privileged)) { + if (!CanNavigate(parsed_url, navigation_constraints)) { DLOG(ERROR) << __FUNCTION__ << " Not allowing navigation to: " << url; return false; } diff --git a/chrome_frame/chrome_frame_automation.h b/chrome_frame/chrome_frame_automation.h index cb1b282..895c9ca 100644 --- a/chrome_frame/chrome_frame_automation.h +++ b/chrome_frame/chrome_frame_automation.h @@ -32,6 +32,7 @@ const unsigned long kCommandExecutionTimeout = 60000; // NOLINT, 60 seconds class ProxyFactory; +class NavigationConstraints; enum AutomationPageFontSize; struct DECLSPEC_NOVTABLE ChromeFrameAutomationProxy { // NOLINT @@ -330,9 +331,11 @@ class ChromeFrameAutomationClient void Uninitialize(); void NotifyAndUninitialize(); - virtual bool InitiateNavigation(const std::string& url, - const std::string& referrer, - bool is_privileged); + virtual bool InitiateNavigation( + const std::string& url, + const std::string& referrer, + NavigationConstraints* navigation_constraints); + virtual bool NavigateToIndex(int index); bool ForwardMessageFromExternalHost(const std::string& message, const std::string& origin, diff --git a/chrome_frame/chrome_frame_npapi.cc b/chrome_frame/chrome_frame_npapi.cc index c645ee8..639a40a 100644 --- a/chrome_frame/chrome_frame_npapi.cc +++ b/chrome_frame/chrome_frame_npapi.cc @@ -877,7 +877,7 @@ void ChromeFrameNPAPI::OnAutomationServerReady() { navigate_after_initialization_ = false; if (!automation_client_->InitiateNavigation(src_, GetDocumentUrl(), - is_privileged_)) { + this)) { DLOG(ERROR) << "Failed to navigate to: " << src_; src_.clear(); } @@ -1168,7 +1168,7 @@ bool ChromeFrameNPAPI::NavigateToURL(const NPVariant* args, uint32_t arg_count, if (ready_state_ == READYSTATE_COMPLETE) { if (!automation_client_->InitiateNavigation(full_url, GetDocumentUrl(), - is_privileged_)) { + this)) { // TODO(tommi): call NPN_SetException. src_.clear(); return false; diff --git a/chrome_frame/chrome_frame_npapi.h b/chrome_frame/chrome_frame_npapi.h index f05185c..3a9e61e 100644 --- a/chrome_frame/chrome_frame_npapi.h +++ b/chrome_frame/chrome_frame_npapi.h @@ -11,6 +11,7 @@ #include "chrome_frame/chrome_frame_automation.h" #include "chrome_frame/chrome_frame_plugin.h" +#include "chrome_frame/navigation_constraints.h" #include "chrome_frame/np_browser_functions.h" #include "chrome_frame/np_event_listener.h" #include "chrome_frame/np_proxy_service.h" @@ -27,7 +28,8 @@ class nsIURI; class ChromeFrameNPAPI : public CWindowImpl<ChromeFrameNPAPI>, public ChromeFramePlugin<ChromeFrameNPAPI>, - public NpEventDelegate { + public NpEventDelegate, + public NavigationConstraintsImpl { public: typedef ChromeFramePlugin<ChromeFrameNPAPI> Base; diff --git a/chrome_frame/external_tab.cc b/chrome_frame/external_tab.cc index ca402d9..ef5fe8c 100644 --- a/chrome_frame/external_tab.cc +++ b/chrome_frame/external_tab.cc @@ -8,6 +8,7 @@ #include "base/task.h" #include "base/waitable_event.h" #include "chrome/common/automation_messages.h" +#include "chrome_frame/chrome_frame_delegate.h" #include "chrome_frame/utils.h" DISABLE_RUNNABLE_METHOD_REFCOUNT(ExternalTabProxy); @@ -121,11 +122,12 @@ void ExternalTabProxy::UiPeerLost(ChromeProxy* proxy, DisconnectReason reason) { } void ExternalTabProxy::Navigate(const std::string& url, - const std::string& referrer, bool is_privileged) { + const std::string& referrer, + NavigationConstraints* navigation_constraints) { // in ui thread // Catch invalid URLs early. Can we allow this navigation to happen? GURL parsed_url(url); - if (!CanNavigate(parsed_url, security_manager_, is_privileged)) { + if (!CanNavigate(parsed_url, navigation_constraints)) { DLOG(ERROR) << __FUNCTION__ << " Not allowing navigation to: " << url; return; } diff --git a/chrome_frame/external_tab.h b/chrome_frame/external_tab.h index 8affbc1..469d2d7 100644 --- a/chrome_frame/external_tab.h +++ b/chrome_frame/external_tab.h @@ -63,6 +63,8 @@ struct CreateTabParams { GURL referrer; }; +class NavigationConstraints; + ///////////////////////////////////////////////////////////////////////// // ExternalTabProxy is a mediator between ChromeProxy (which runs mostly in // background IPC-channel thread and the UI object (ActiveX, NPAPI, @@ -92,7 +94,7 @@ class ExternalTabProxy : public CWindowImpl<ExternalTabProxy>, virtual void CreateTab(const CreateTabParams& create_params, UIDelegate* delegate); virtual void Navigate(const std::string& url, const std::string& referrer, - bool is_privileged); + NavigationConstraints* navigation_constraints); virtual void NavigateToIndex(int index); virtual void ForwardMessageFromExternalHost(const std::string& message, const std::string& origin, const std::string& target); @@ -218,8 +220,6 @@ class ExternalTabProxy : public CWindowImpl<ExternalTabProxy>, referrer = ref; } } pending_navigation_; - - ScopedComPtr<IInternetSecurityManager> security_manager_; }; #endif // CHROME_FRAME_EXTERNAL_TAB_H_ diff --git a/chrome_frame/external_tab_test.cc b/chrome_frame/external_tab_test.cc index 5e3e44a..ac3359c 100644 --- a/chrome_frame/external_tab_test.cc +++ b/chrome_frame/external_tab_test.cc @@ -10,10 +10,11 @@ // #include "base/waitable_event.h" #include "chrome/common/automation_messages.h" +#include "chrome_frame/navigation_constraints.h" +#include "chrome_frame/test/chrome_frame_test_utils.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock_mutant.h" -#include "chrome_frame/test/chrome_frame_test_utils.h" @@ -236,8 +237,11 @@ TEST(ExternalTabProxy, NavigateAfterCreate) { tab_params.is_widget_mode = false; tab_params.url = initial_url; + NavigationConstraintsImpl navigation_constraints; + tab->CreateTab(tab_params, &ui_delegate); - tab->Navigate("http://asgard.org", EmptyString(), true); + tab->Navigate("http://asgard.org", EmptyString(), + &navigation_constraints); loop.RunFor(5); EXPECT_FALSE(loop.WasTimedOut()); diff --git a/chrome_frame/navigation_constraints.cc b/chrome_frame/navigation_constraints.cc new file mode 100644 index 0000000..0845bb1 --- /dev/null +++ b/chrome_frame/navigation_constraints.cc @@ -0,0 +1,69 @@ +// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome_frame/navigation_constraints.h" + +#include "base/string_util.h" +#include "base/utf_string_conversions.h" +#include "chrome/common/url_constants.h" +#include "chrome_frame/utils.h" + +// NavigationConstraintsImpl method definitions. +bool NavigationConstraintsImpl::AllowUnsafeUrls() { + // No sanity checks if unsafe URLs are allowed + return GetConfigBool(false, kAllowUnsafeURLs); +} + +bool NavigationConstraintsImpl::IsSchemeAllowed(const GURL& url) { + if (url.is_empty()) + return false; + + if (!url.is_valid()) + return false; + + if (url.SchemeIs(chrome::kHttpScheme) || + url.SchemeIs(chrome::kHttpsScheme)) + return true; + + // Additional checking for view-source. Allow only http and https + // URLs in view source. + if (url.SchemeIs(chrome::kViewSourceScheme)) { + GURL sub_url(url.path()); + if (sub_url.SchemeIs(chrome::kHttpScheme) || + sub_url.SchemeIs(chrome::kHttpsScheme)) + return true; + } + + // Allow only about:blank or about:version + if (url.SchemeIs(chrome::kAboutScheme)) { + if (LowerCaseEqualsASCII(url.spec(), chrome::kAboutBlankURL) || + LowerCaseEqualsASCII(url.spec(), chrome::kAboutVersionURL)) { + return true; + } + } + return false; +} + +bool NavigationConstraintsImpl::IsZoneAllowed(const GURL& url) { + if (!security_manager_) { + HRESULT hr = security_manager_.CreateInstance( + CLSID_InternetSecurityManager); + if (FAILED(hr)) { + NOTREACHED() << __FUNCTION__ + << " Failed to create SecurityManager. Error: 0x%x" + << hr; + return true; + } + 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 restricted url: " << url; + return false; + } + } + return true; +} + diff --git a/chrome_frame/navigation_constraints.h b/chrome_frame/navigation_constraints.h new file mode 100644 index 0000000..a66cdeb --- /dev/null +++ b/chrome_frame/navigation_constraints.h @@ -0,0 +1,37 @@ +// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_FRAME_NAVIGATION_CONSTRAINTS_H_ +#define CHROME_FRAME_NAVIGATION_CONSTRAINTS_H_ + +#include <urlmon.h> +#include <windows.h> + +#include "base/win/scoped_comptr.h" +#include "googleurl/src/gurl.h" + +// Provides an interface which controls navigation within ChromeFrame. +class NavigationConstraints { + public: + virtual ~NavigationConstraints() {} + virtual bool AllowUnsafeUrls() = 0; + virtual bool IsSchemeAllowed(const GURL& url) = 0; + virtual bool IsZoneAllowed(const GURL& url) = 0; +}; + +// Provides default implementation for the NavigationConstraints interface. +class NavigationConstraintsImpl : public NavigationConstraints { + public: + virtual ~NavigationConstraintsImpl() {} + + // NavigationConstraints method overrides. + virtual bool AllowUnsafeUrls(); + virtual bool IsSchemeAllowed(const GURL& url); + virtual bool IsZoneAllowed(const GURL& url); + private: + base::win::ScopedComPtr<IInternetSecurityManager> security_manager_; +}; + +#endif // CHROME_FRAME_NAVIGATION_CONSTRAINTS_H_ + diff --git a/chrome_frame/test/automation_client_mock.cc b/chrome_frame/test/automation_client_mock.cc index c02d7e0d..729ecbd 100644 --- a/chrome_frame/test/automation_client_mock.cc +++ b/chrome_frame/test/automation_client_mock.cc @@ -4,9 +4,10 @@ #include "chrome_frame/test/automation_client_mock.h" #include "base/callback.h" -#include "net/base/net_errors.h" #include "chrome_frame/custom_sync_call_context.h" +#include "chrome_frame/navigation_constraints.h" #include "chrome_frame/test/chrome_frame_test_utils.h" +#include "net/base/net_errors.h" #define GMOCK_MUTANT_INCLUDE_LATE_OBJECT_BINDING #include "testing/gmock_mutant.h" @@ -85,8 +86,8 @@ ACTION_P4(HandleCreateTab, tab_handle, external_tab_container, tab_wnd, delete context; } -ACTION_P4(InitiateNavigation, client, url, referrer, privileged) { - client->InitiateNavigation(url, referrer, privileged); +ACTION_P4(InitiateNavigation, client, url, referrer, constraints) { + client->InitiateNavigation(url, referrer, constraints); } // We mock ChromeFrameDelegate only. The rest is with real AutomationProxy @@ -151,6 +152,8 @@ TEST(CFACWithChrome, CreateNotSoFast) { TEST(CFACWithChrome, NavigateOk) { MockCFDelegate cfd; + NavigationConstraintsImpl navigation_constraints; + chrome_frame_test::TimedMsgLoop loop; const std::string url = "about:version"; const FilePath profile_path( @@ -162,7 +165,8 @@ TEST(CFACWithChrome, NavigateOk) { EXPECT_CALL(cfd, OnAutomationServerReady()) .WillOnce(InitiateNavigation(client.get(), - url, std::string(), false)); + url, std::string(), + &navigation_constraints)); EXPECT_CALL(cfd, GetBounds(_)).Times(testing::AnyNumber()); @@ -196,6 +200,7 @@ TEST(CFACWithChrome, NavigateOk) { TEST(CFACWithChrome, NavigateFailed) { MockCFDelegate cfd; + NavigationConstraintsImpl navigation_constraints; chrome_frame_test::TimedMsgLoop loop; const FilePath profile_path( chrome_frame_test::GetProfilePath(L"Adam.N.Epilinter")); @@ -210,7 +215,7 @@ TEST(CFACWithChrome, NavigateFailed) { EXPECT_CALL(cfd, OnAutomationServerReady()) .WillOnce(testing::IgnoreResult(testing::InvokeWithoutArgs(CreateFunctor( client.get(), &ChromeFrameAutomationClient::InitiateNavigation, - url, std::string(), false)))); + url, std::string(), &navigation_constraints)))); EXPECT_CALL(cfd, GetBounds(_)).Times(testing::AnyNumber()); EXPECT_CALL(cfd, OnNavigationStateChanged(_, _)).Times(testing::AnyNumber()); @@ -419,6 +424,8 @@ TEST_F(CFACMockTest, OnChannelError) { TEST_F(CFACMockTest, NavigateTwiceAfterInitToSameUrl) { int timeout = 500; + NavigationConstraintsImpl navigation_constraints; + CreateTab(); SetAutomationServerOk(1); @@ -439,7 +446,7 @@ TEST_F(CFACMockTest, NavigateTwiceAfterInitToSameUrl) { EXPECT_CALL(cfd_, OnAutomationServerReady()) .WillOnce(InitiateNavigation(client_.get(), std::string("http://www.nonexistent.com"), - std::string(), false)); + std::string(), &navigation_constraints)); EXPECT_CALL(mock_proxy_, SendAsAsync(testing::Property( &IPC::SyncMessage::type, AutomationMsg_NavigateInExternalTab::ID), diff --git a/chrome_frame/test/util_unittests.cc b/chrome_frame/test/util_unittests.cc index 0964296..68173b9 100644 --- a/chrome_frame/test/util_unittests.cc +++ b/chrome_frame/test/util_unittests.cc @@ -5,7 +5,10 @@ #include "base/file_path.h" #include "base/file_version_info.h" #include "base/file_version_info_win.h" +#include "base/string_util.h" +#include "base/utf_string_conversions.h" #include "base/win/registry.h" +#include "chrome_frame/navigation_constraints.h" #include "chrome_frame/test/chrome_frame_test_utils.h" #include "chrome_frame/utils.h" @@ -228,8 +231,36 @@ class MockIInternetSecurityManager : public IInternetSecurityManager { HRESULT(DWORD zone, IEnumString** enum_string, DWORD flags)); }; +// This class provides a partial mock for the NavigationConstraints +// interface by providing specialized zone overrides. +class MockNavigationConstraintsZoneOverride + : public NavigationConstraintsImpl { + public: + MOCK_METHOD1(IsZoneAllowed, bool(const GURL&url)); +}; + +// Mock NavigationConstraints +class MockNavigationConstraints : public NavigationConstraints { + public: + MOCK_METHOD0(AllowUnsafeUrls, bool()); + MOCK_METHOD1(IsSchemeAllowed, bool(const GURL& url)); + MOCK_METHOD1(IsZoneAllowed, bool(const GURL& url)); +}; + +// Matcher which returns true if the URL passed in starts with the prefix +// specified. +MATCHER_P(UrlPathStartsWith, url_prefix, "url starts with prefix") { + return StartsWith(UTF8ToWide(arg.spec()), url_prefix, false); +} + +ACTION_P3(HandleZone, mock, url_prefix, zone) { + if (StartsWith(UTF8ToWide(arg0.spec()), url_prefix, false)) + return zone != URLZONE_UNTRUSTED; + return false; +} + TEST_F(UtilTests, CanNavigateTest) { - MockIInternetSecurityManager mock; + MockNavigationConstraintsZoneOverride mock; struct Zones { const wchar_t* url_prefix; @@ -247,76 +278,139 @@ TEST_F(UtilTests, CanNavigateTest) { 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_CALL(mock, IsZoneAllowed(UrlPathStartsWith(zone.url_prefix))) + .WillRepeatedly(testing::Return(zone.zone != URLZONE_UNTRUSTED)); } 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 }, + { " ", false, false }, + { "foo bar", false, false }, // non-privileged test cases - { "http://blah/?attach_external_tab&10&1&0&0&100&100&iexplore", false, - true, true }, - { "http://untrusted/bar.html", false, false, true }, - { "http://blah/?attach_external_tab&10&1&0&0&100&100&iexplore", 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&iexplore", true, true, - true }, - { "http://untrusted/bar.html", true, false, true }, - { "http://blah/?attach_external_tab&10&1&0&0&100&100&iexplore", 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 }, + { "http://blah/?attach_external_tab&10&1&0&0&100&100&iexplore", true, + true }, + { "http://untrusted/bar.html", false, true }, + { "http://blah/?attach_external_tab&10&1&0&0&100&100&iexplore", true, + true }, + { "view-source:http://www.google.ca", true, true }, + { "view-source:javascript:alert('foo');", false, true }, + { "about:blank", true, true }, + { "About:Version", true, true }, + { "about:config", false, true }, + { "chrome-extension://aaaaaaaaaaaaaaaaaaa/toolstrip.html", false, true }, + { "ftp://www.google.ca", false, true }, + { "file://www.google.ca", false, true }, + { "file://C:\boot.ini", false, true }, + { "SIP:someone@10.1.2.3", false, 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); + bool actual = CanNavigate(GURL(test.url), &mock); EXPECT_EQ(test.default_expected, actual) << "Failure url: " << test.url; } +} - bool enable_gcf = GetConfigBool(false, kAllowUnsafeURLs); - SetConfigBool(kAllowUnsafeURLs, true); +TEST_F(UtilTests, CanNavigateTestDenyAll) { + MockNavigationConstraints 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; + EXPECT_CALL(mock, IsZoneAllowed(testing::_)) + .Times(testing::AnyNumber()) + .WillRepeatedly(testing::Return(false)); + + EXPECT_CALL(mock, IsSchemeAllowed(testing::_)) + .Times(testing::AnyNumber()) + .WillRepeatedly(testing::Return(false)); + + EXPECT_CALL(mock, AllowUnsafeUrls()) + .Times(testing::AnyNumber()) + .WillRepeatedly(testing::Return(false)); + + char *urls[] = { + { " "}, + { "foo bar"}, + // non-privileged test cases + { "http://blah/?attach_external_tab&10&1&0&0&100&100&iexplore"}, + { "http://untrusted/bar.html"}, + { "http://blah/?attach_external_tab&10&1&0&0&100&100&iexplore"}, + { "view-source:http://www.google.ca"}, + { "view-source:javascript:alert('foo');"}, + { "about:blank"}, + { "About:Version"}, + { "about:config"}, + { "chrome-extension://aaaaaaaaaaaaaaaaaaa/toolstrip.html"}, + { "ftp://www.google.ca"}, + { "file://www.google.ca"}, + { "file://C:\boot.ini"}, + { "SIP:someone@10.1.2.3"}, + }; + + for (int i = 0; i < arraysize(urls); ++i) { + EXPECT_FALSE(CanNavigate(GURL(urls[i]), &mock)); } +} + +TEST_F(UtilTests, CanNavigateTestAllowAll) { + MockNavigationConstraints mock; - SetConfigBool(kAllowUnsafeURLs, enable_gcf); + EXPECT_CALL(mock, AllowUnsafeUrls()) + .Times(testing::AnyNumber()) + .WillRepeatedly(testing::Return(false)); + + EXPECT_CALL(mock, IsSchemeAllowed(testing::_)) + .Times(testing::AnyNumber()) + .WillRepeatedly(testing::Return(true)); + + EXPECT_CALL(mock, IsZoneAllowed(testing::_)) + .Times(testing::AnyNumber()) + .WillRepeatedly(testing::Return(true)); + + char *urls[] = { + // non-privileged test cases + { "http://blah/?attach_external_tab&10&1&0&0&100&100&iexplore"}, + { "http://untrusted/bar.html"}, + { "http://blah/?attach_external_tab&10&1&0&0&100&100&iexplore"}, + { "view-source:http://www.google.ca"}, + { "view-source:javascript:alert('foo');"}, + { "about:blank"}, + { "About:Version"}, + { "about:config"}, + { "chrome-extension://aaaaaaaaaaaaaaaaaaa/toolstrip.html"}, + { "ftp://www.google.ca"}, + { "file://www.google.ca"}, + { "file://C:\boot.ini"}, + { "SIP:someone@10.1.2.3"}, + { "gcf:about:cache"}, + { "gcf:about:plugins"}, + }; + + for (int i = 0; i < arraysize(urls); ++i) { + EXPECT_TRUE(CanNavigate(GURL(urls[i]), &mock)); + } +} + +TEST_F(UtilTests, CanNavigateTestAllowAllUnsafeUrls) { + MockNavigationConstraints mock; + + EXPECT_CALL(mock, AllowUnsafeUrls()) + .Times(testing::AnyNumber()) + .WillRepeatedly(testing::Return(true)); + + char *urls[] = { + {"gcf:about:cache"}, + {"gcf:http://www.google.com"}, + {"view-source:javascript:alert('foo');"}, + {"http://www.google.com"}, + }; + + for (int i = 0; i < arraysize(urls); ++i) { + EXPECT_TRUE(CanNavigate(GURL(urls[i]), &mock)); + } } TEST_F(UtilTests, IsDefaultRendererTest) { diff --git a/chrome_frame/utils.cc b/chrome_frame/utils.cc index f1495e8..c74a840 100644 --- a/chrome_frame/utils.cc +++ b/chrome_frame/utils.cc @@ -31,6 +31,7 @@ #include "chrome/installer/util/chrome_frame_distribution.h" #include "chrome_frame/extra_system_apis.h" #include "chrome_frame/html_utils.h" +#include "chrome_frame/navigation_constraints.h" #include "chrome_frame/policy_settings.h" #include "chrome_frame/simple_resource_loader.h" #include "googleurl/src/gurl.h" @@ -1423,44 +1424,32 @@ void ChromeFrameUrl::Reset() { profile_name_.clear(); } -bool CanNavigate(const GURL& url, IInternetSecurityManager* security_manager, - bool is_privileged) { +bool CanNavigate(const GURL& url, + NavigationConstraints* navigation_constraints) { if (!url.is_valid()) { DLOG(ERROR) << "Invalid URL passed to InitiateNavigation: " << url; return false; } + if (!navigation_constraints) { + NOTREACHED() << "Invalid NavigationConstraints passed in"; + return false; + } + // No sanity checks if unsafe URLs are allowed - if (GetConfigBool(false, kAllowUnsafeURLs)) + if (navigation_constraints->AllowUnsafeUrls()) return true; - if (!IsValidUrlScheme(url, is_privileged)) { + if (!navigation_constraints->IsSchemeAllowed(url)) { 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 about url: " << url; - return false; - } - } - - // 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 restricted url: " << url; - return false; - } + if (!navigation_constraints->IsZoneAllowed(url)) { + DLOG(WARNING) << __FUNCTION__ + << " Disallowing navigation to restricted url: " << url; + return false; } - return true; } diff --git a/chrome_frame/utils.h b/chrome_frame/utils.h index 4492078..2e167d2 100644 --- a/chrome_frame/utils.h +++ b/chrome_frame/utils.h @@ -564,12 +564,12 @@ class ChromeFrameUrl { std::string profile_name_; }; +class NavigationConstraints; // 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 CanNavigate(const GURL& url, IInternetSecurityManager* security_manager, - bool is_privileged); +// These decisions are controlled by the NavigationConstraints object passed +// in. +bool CanNavigate(const GURL& url, + NavigationConstraints* navigation_constraints); // Utility function that prevents the current module from ever being unloaded. // Call if you make irreversible patches. |