<webview>: BrowserPluginHostMsg_RespondPermission IPC cleanup

Carrying around the permission type from the embedder to the browser process
seems redundant. In fact, it also allows for a compromised app to crash the
browser process: It can make a geolocation request, but pretend the response
is something other than a GeolocationRequest. The request object in
BrowserPluginGuest removes the request from the map, and the iterator becomes
invalid. When we try to delete the request and remove the iterator, the browser
process will crash.

BUG=166165
Test=WebViewTest.*
TBR=cdn@chromium.org for trivial browser_plugin_messages.h change.

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

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

4 files changed, 17 insertions, 9 deletions

diff --git a/content/browser/browser_plugin/browser_plugin_guest.cc b/content/browser/browser_plugin/browser_plugin_guest.cc
index 4027365..2bcb301 100644
--- a/content/browser/browser_plugin/browser_plugin_guest.cc
+++ b/content/browser/browser_plugin/browser_plugin_guest.cc
@@ -63,6 +63,7 @@ class BrowserPluginGuest::PermissionRequest {
  public:
   virtual void Respond(bool should_allow) = 0;
   virtual ~PermissionRequest() {}
+  virtual BrowserPluginPermissionType GetType() const = 0;
  protected:
   PermissionRequest() {
     RecordAction(UserMetricsAction("BrowserPlugin.Guest.PermissionRequest"));
@@ -79,6 +80,9 @@ class BrowserPluginGuest::DownloadRequest : public PermissionRequest {
   virtual void Respond(bool should_allow) OVERRIDE {
     callback_.Run(should_allow);
   }
+  virtual BrowserPluginPermissionType GetType() const OVERRIDE {
+    return BrowserPluginPermissionTypeDownload;
+  }
   virtual ~DownloadRequest() {}
  private:
   base::Callback<void(bool)> callback_;
@@ -129,6 +133,9 @@ class BrowserPluginGuest::GeolocationRequest : public PermissionRequest {
     }
     guest_->SetGeolocationPermission(callback_, bridge_id_, false);
   }
+  virtual BrowserPluginPermissionType GetType() const OVERRIDE {
+    return BrowserPluginPermissionTypeGeolocation;
+  }
   virtual ~GeolocationRequest() {}
  private:
   base::Callback<void(bool)> callback_;
@@ -159,7 +166,9 @@ class BrowserPluginGuest::MediaRequest : public PermissionRequest {
       // Deny the request.
       callback_.Run(MediaStreamDevices(), scoped_ptr<MediaStreamUI>());
     }
-
+  }
+  virtual BrowserPluginPermissionType GetType() const OVERRIDE {
+    return BrowserPluginPermissionTypeMedia;
   }
   virtual ~MediaRequest() {}
  private:
@@ -192,6 +201,9 @@ class BrowserPluginGuest::NewWindowRequest : public PermissionRequest {
     if (!should_allow)
       guest->Destroy();
   }
+  virtual BrowserPluginPermissionType GetType() const OVERRIDE {
+    return BrowserPluginPermissionTypeNewWindow;
+  }
   virtual ~NewWindowRequest() {}
  private:
   int instance_id_;
@@ -1290,7 +1302,6 @@ void BrowserPluginGuest::OnSetVisibility(int instance_id, bool visible) {
 
 void BrowserPluginGuest::OnRespondPermission(
     int instance_id,
-    BrowserPluginPermissionType permission_type,
     int request_id,
     bool should_allow) {
   RequestMap::iterator request_itr = permission_request_map_.find(request_id);
@@ -1298,6 +1309,7 @@ void BrowserPluginGuest::OnRespondPermission(
     LOG(INFO) << "Not a valid request ID.";
     return;
   }
+  BrowserPluginPermissionType permission_type = request_itr->second->GetType();
   request_itr->second->Respond(should_allow);
 
   // Geolocation requests have to hang around for a while, so we don't delete
@@ -1485,8 +1497,7 @@ void BrowserPluginGuest::DidRetrieveDownloadURLFromRequestId(
     int permission_request_id,
     const std::string& url) {
   if (url.empty()) {
-    OnRespondPermission(instance_id(), BrowserPluginPermissionTypeDownload,
-                        permission_request_id, false);
+    OnRespondPermission(instance_id(), permission_request_id, false);
     return;
   }
 
diff --git a/content/browser/browser_plugin/browser_plugin_guest.h b/content/browser/browser_plugin/browser_plugin_guest.h
index f30e148..642db2b 100644
--- a/content/browser/browser_plugin/browser_plugin_guest.h
+++ b/content/browser/browser_plugin/browser_plugin_guest.h
@@ -304,7 +304,6 @@ class CONTENT_EXPORT BrowserPluginGuest
   // Allows or denies a permission request access, after the embedder has had a
   // chance to decide.
   void OnRespondPermission(int instance_id,
-                           BrowserPluginPermissionType permission_type,
                            int request_id,
                            bool should_allow);
   // Handles drag events from the embedder.
diff --git a/content/common/browser_plugin/browser_plugin_messages.h b/content/common/browser_plugin/browser_plugin_messages.h
index a8f4d01..94add96 100644
--- a/content/common/browser_plugin/browser_plugin_messages.h
+++ b/content/common/browser_plugin/browser_plugin_messages.h
@@ -251,9 +251,8 @@ IPC_MESSAGE_ROUTED2(BrowserPluginHostMsg_SetName,
 // permission, since a security check in the embedder might follow. For example
 // for media access permission, the guest will be granted permission only if its
 // embedder also has access.
-IPC_MESSAGE_ROUTED4(BrowserPluginHostMsg_RespondPermission,
+IPC_MESSAGE_ROUTED3(BrowserPluginHostMsg_RespondPermission,
                     int /* instance_id */,
-                    BrowserPluginPermissionType /* permission_type */,
                     int /* request_id */,
                     bool /* allow */)
 
diff --git a/content/renderer/browser_plugin/browser_plugin.cc b/content/renderer/browser_plugin/browser_plugin.cc
index bca01bd..326fdcd 100644
--- a/content/renderer/browser_plugin/browser_plugin.cc
+++ b/content/renderer/browser_plugin/browser_plugin.cc
@@ -969,8 +969,7 @@ void BrowserPlugin::RespondPermission(
   else
     browser_plugin_manager()->Send(
         new BrowserPluginHostMsg_RespondPermission(
-            render_view_routing_id_, guest_instance_id_, permission_type,
-            request_id, allow));
+            render_view_routing_id_, guest_instance_id_, request_id, allow));
 }
 
 void BrowserPlugin::RespondPermissionPointerLock(bool allow) {