Kill renderer processes that fail check in RenderViewHostImpl::SetWebUIProperty

When setting WebUI property for a renderer, it should have WebUI bindings enabled. If we get into inconsistent state where the renderer doesn't have those bindings, but we are trying to set WebUI properties, we should kill it. This is a defense in depth measure against security exploits.

BUG=174063

Review URL: https://chromiumcodereview.appspot.com/12210038

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

4 files changed, 89 insertions, 6 deletions

diff --git a/chrome/tools/chromeactions.txt b/chrome/tools/chromeactions.txt
index 2abe98b..30c12c8 100644
--- a/chrome/tools/chromeactions.txt
+++ b/chrome/tools/chromeactions.txt
@@ -125,6 +125,7 @@
 0x554b7c860c749c2f	BadMessageTerminate_ACDH
 0x878b28b309d1205e	BadMessageTerminate_AOF
 0xec6518c4af50b7ac	BadMessageTerminate_BMF
+0x1f57dc66c6c91837	BadMessageTerminate_BPE
 0x5a858938e484c903	BadMessageTerminate_BRPH
 0x6f41bf748eb54008	BadMessageTerminate_DBMF
 0xd910b7f4e1b53c11	BadMessageTerminate_DSMF
@@ -145,6 +146,7 @@
 0xa00e08812a4284c2	BadMessageTerminate_RWH4
 0xefc9deffa33ee67d	BadMessageTerminate_RWH5
 0xc4874f0e8e8b60aa	BadMessageTerminate_WPH
+0x56649dd19258ed1f	BindingsMismatchTerminate_RVH_WebUI
 0x1d145f0af708242c	BlockNonsandboxedPlugins_Disable
 0xd80cc9291c9c82a9	BlockNonsandboxedPlugins_Enable
 0xe0daa169d443430e	BlockedPluginInfobar.AllowThisTime
@@ -222,8 +224,11 @@
 0xde8be5ce26955605	BrowserPlugin.Guest.Hung
 0xa369e99aa2e21969	BrowserPlugin.Guest.Killed
 0x428cf267aeb35e28	BrowserPlugin.Guest.Navigate
+0x69b219de7f17c077	BrowserPlugin.Guest.Responsive
 0x7784cf1f8b1cc3f0	BrowserPlugin.Guest.Terminate
 0xea4788705e6873b4	Cancel
+0xb1c07c66ce4ae2ac	Caption_ClickTogglesMaximize
+0x96c3ac2d2a5d9dba	Caption_GestureTogglesMaximize
 0x89394b102e55da81	ClearAuthenticationCache
 0x6bd5f5b094096aa7	ClearBrowsingData_Autofill
 0xae5b20986fb024db	ClearBrowsingData_Cache
@@ -290,6 +295,8 @@
 0xd3e90631d6d04d51	DevTools_InspectElement
 0xbadaf91b6bdbbe68	DevTools_ToggleConsole
 0xddaad2f5e9238157	DevTools_ToggleWindow
+0xe581401517f920ca	DisabledExtensionNotificationDismissed
+0x240b0da0a404d35c	DisabledExtensionNotificationShown
 0xdad0f491267f672e	DockingWindow_Bottom
 0x7ecb78846fadf9bf	DockingWindow_BottomHalf
 0xc818526e20834ebf	DockingWindow_Left
@@ -946,6 +953,17 @@
 0x31374d163aec5a5e	Login_GuestLoginSuccess
 0x47421e3d3406b4e1	Login_OffTheRecordLoginSuccess
 0xc23fa875d14a7ddb	Login_Success
+0x41b5faabb7c9327c	ManagedMode_MainFrameNavigation
+0xa29f3f7e4bb25494	ManagedMode_NewManagedUserWindow
+0x238d1563c0fa1ce2	ManagedMode_OpenSettings
+0x9f5c6206cd6609b6	ManagedMode_StartupManagedSwitch
+0x91519377450fa09e	ManagedMode_StartupNoManagedSwitch
+0x9c4e110de24ddbfb	MaxButton_MaxLeft
+0xe5e2c8bb60a6f019	MaxButton_MaxRight
+0x0ed29608c3edb9ee	MaxButton_Maximize
+0xfa675ab4e35a8dfb	MaxButton_Minimize
+0x9ddc8fc34f81c18c	MaxButton_Restore
+0xf5f4e08ff4ffc48e	MaxButton_ShowBubble
 0x84ba0ed3cbdf3956	MediaContextMenu_Controls
 0x7b82a108ac28a1ac	MediaContextMenu_Loop
 0x458edb8f0451b9f5	MediaContextMenu_Mute
@@ -1083,12 +1101,14 @@
 0x95c990454684cb1d	NewTabPage_ReopenTab
 0xab4d417c5ca44904	NewTab_Button
 0xbdc9ec125e7a3ade	NewWindow
+0x268376698078c71b	OmniboxInputInProgress
 0xe7ff15c3f1043a26	Omnibox_DragString
 0x1a18c36c737ec22b	Omnibox_DragURL
 0x56c5e8af805a2fe8	OpenAddBluetoothDeviceDialog
 0xa00fbd8da8229c83	OpenAllBookmarks
 0x7242962875070018	OpenAllBookmarksIncognitoWindow
 0x5e3bd4e3535ecc38	OpenAllBookmarksNewWindow
+0xf6bce188756ecaf8	OpenChangeProfilePictureDialog
 0x4b858349a1b8bb15	OpenFile
 0xedaa8487de2a33c6	OpenFileManager
 0xb3c3e8d99702cf70	OpenFileSystemPersistent
@@ -1128,6 +1148,7 @@
 0x3f92cd6678d2f595	Options_DefaultHandlersSettingChanged
 0x5dfe307474e6b526	Options_DefaultImagesSettingChanged
 0x8ac0134529158dae	Options_DefaultJavaScriptSettingChanged
+0x04303682ca0b2a8d	Options_DefaultMediaStreamMicSettingChanged
 0x6a97ed68e3457d0e	Options_DefaultMediaStreamSettingChanged
 0xfca02a749fa0f811	Options_DefaultMouseLockSettingChanged
 0xbc49f9107e7c7c7c	Options_DefaultNotificationsSettingChanged
@@ -1480,6 +1501,8 @@
 0x34a770eb3bbf5632	WP_Gallery
 0x949730a9468e27a1	WebsiteSettings_CookiesDialogOpened
 0xddc2a5698e145d16	WebsiteSettings_Opened
+0x75af94f65efafada	Win8DesktopRestart
+0x8f88175ece0f933b	Win8MetroRestart
 0x554103fbf5582ee0	ZoomMinus
 0x82d278b1f2e78bcd	ZoomMinus_AtMinimum
 0x4344cd22d03f6800	ZoomNormal
diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc
index bbf36ca..b01d378 100644
--- a/content/browser/renderer_host/render_view_host_impl.cc
+++ b/content/browser/renderer_host/render_view_host_impl.cc
@@ -816,13 +816,17 @@ int RenderViewHostImpl::GetEnabledBindings() const {
 
 void RenderViewHostImpl::SetWebUIProperty(const std::string& name,
                                           const std::string& value) {
-  // This is just a sanity check before telling the renderer to enable the
-  // property.  It could lie and send the corresponding IPC messages anyway,
-  // but we will not act on them if enabled_bindings_ doesn't agree.
-  if (enabled_bindings_ & BINDINGS_POLICY_WEB_UI)
+  // This is a sanity check before telling the renderer to enable the property.
+  // It could lie and send the corresponding IPC messages anyway, but we will
+  // not act on them if enabled_bindings_ doesn't agree. If we get here without
+  // WebUI bindings, kill the renderer process.
+  if (enabled_bindings_ & BINDINGS_POLICY_WEB_UI) {
     Send(new ViewMsg_SetWebUIProperty(GetRoutingID(), name, value));
-  else
-    NOTREACHED() << "WebUI bindings not enabled.";
+  } else {
+    RecordAction(UserMetricsAction("BindingsMismatchTerminate_RVH_WebUI"));
+    base::KillProcess(
+        GetProcess()->GetHandle(), content::RESULT_CODE_KILLED, false);
+  }
 }
 
 void RenderViewHostImpl::GotFocus() {
diff --git a/content/browser/security_exploit_browsertest.cc b/content/browser/security_exploit_browsertest.cc
new file mode 100644
index 0000000..bbe7b2e
--- /dev/null
+++ b/content/browser/security_exploit_browsertest.cc
@@ -0,0 +1,55 @@
+// Copyright (c) 2013 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 "base/command_line.h"
+#include "content/browser/renderer_host/render_view_host_impl.h"
+#include "content/browser/web_contents/web_contents_impl.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_types.h"
+#include "content/public/common/content_switches.h"
+#include "content/public/test/test_utils.h"
+#include "content/shell/shell.h"
+#include "content/test/content_browser_test.h"
+#include "content/test/content_browser_test_utils.h"
+
+namespace content {
+
+// The goal of these tests will be to "simulate" exploited renderer processes,
+// which can send arbitrary IPC messages and confuse browser process internal
+// state, leading to security bugs. We are trying to verify that the browser
+// doesn't perform any dangerous operations in such cases.
+class SecurityExploitBrowserTest : public ContentBrowserTest {
+ public:
+  SecurityExploitBrowserTest() {}
+  virtual void SetUpCommandLine(CommandLine* command_line) {
+    ASSERT_TRUE(test_server()->Start());
+
+    // Add a host resolver rule to map all outgoing requests to the test server.
+    // This allows us to use "real" hostnames in URLs, which we can use to
+    // create arbitrary SiteInstances.
+    command_line->AppendSwitchASCII(
+        switches::kHostResolverRules,
+        "MAP * " + test_server()->host_port_pair().ToString() +
+            ",EXCLUDE localhost");
+  }
+};
+
+// Ensure that we kill the renderer process if we try to give it WebUI
+// properties and it doesn't have enabled WebUI bindings.
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest, SetWebUIProperty) {
+  GURL foo("http://foo.com/files/simple_page.html");
+
+  NavigateToURL(shell(), foo);
+  EXPECT_EQ(0,
+      shell()->web_contents()->GetRenderViewHost()->GetEnabledBindings());
+
+  content::WindowedNotificationObserver terminated(
+      content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
+      content::NotificationService::AllSources());
+  shell()->web_contents()->GetRenderViewHost()->SetWebUIProperty(
+      "toolkit", "views");
+  terminated.Wait();
+}
+
+}
diff --git a/content/content_tests.gypi b/content/content_tests.gypi
index 715fcec..92cb115 100644
--- a/content/content_tests.gypi
+++ b/content/content_tests.gypi
@@ -733,6 +733,7 @@
             'browser/renderer_host/render_view_host_manager_browsertest.cc',
             'browser/renderer_host/render_widget_host_view_browsertest.cc',
             'browser/renderer_host/render_widget_host_view_win_browsertest.cc',
+            'browser/security_exploit_browsertest.cc',
             'browser/session_history_browsertest.cc',
             'browser/site_per_process_browsertest.cc',
             'browser/speech/speech_recognition_browsertest.cc',