Fix uaf in ApplicationInstance.

mojo/shell/application_instance.cc

 // Copyright 2015 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 "mojo/shell/application_instance.h"
 
 #include "base/bind.h"
 #include "base/stl_util.h"
 #include "mojo/application/public/interfaces/content_handler.mojom.h"
 #include "mojo/common/common_type_converters.h"
 #include "mojo/common/url_type_converters.h"
 #include "mojo/shell/application_manager.h"
 
 namespace mojo {
 namespace shell {
 
 ApplicationInstance::QueuedClientRequest::QueuedClientRequest() {
 }
 
 ApplicationInstance::QueuedClientRequest::~QueuedClientRequest() {
 }
 
 ApplicationInstance::ApplicationInstance(
     ApplicationPtr application,
     ApplicationManager* manager,
+    const Identity& originator_identity,
     const Identity& identity,
     const CapabilityFilter& filter,
     const base::Closure& on_application_end)
     : manager_(manager),
+      originator_identity_(originator_identity),
       identity_(identity),
       filter_(filter),
       allow_any_application_(filter.size() == 1 && filter.count("*") == 1),
       on_application_end_(on_application_end),
       application_(application.Pass()),
       binding_(this),
       queue_requests_(false) {
   binding_.set_connection_error_handler([this]() { OnConnectionError(); });
 }
 
@@ skipping 97 matching lines @@
   queued_client_requests_.swap(queued_client_requests);
   auto manager = manager_;
   manager_->OnApplicationInstanceError(this);
   //|this| is deleted.
 
   // If any queued requests came to shell during time it was shutting down,
   // start them now.
   for (auto request : queued_client_requests) {
     mojo::URLRequestPtr url(mojo::URLRequest::New());
     url->url = mojo::String::From(request->requested_url.spec());
-    manager->ConnectToApplication(this, url.Pass(), std::string(),
+    ApplicationInstance* originator =
+        manager->GetApplicationInstance(originator_identity_);

[Ben Goodger (Google), 2015/07/27 19:26:34]

note to my future self: I am concerned that in its current form identity is
insufficient here, as |originator| as returned may differ from the instance that
spawned this instance. e.g.:

Instance1 is created for application identity "A,B" with a specific capability
filter CF1.
Instance1 connects to Instance2.
..
Instance1 is destroyed.
Instance3 is created for application identity "A,B" with specific capability
filter CF2 <-- note the difference.
Instance2 experiences a connection error and this code is reached.  It asks the
application manager for the instance corresponding to "A,B" and gets Instance3,
which has a different set of allowed interfaces than long-gone Instance1. Oops.

Ultimately I think CF should be part of the application identity.

+    manager->ConnectToApplication(originator, url.Pass(), std::string(),
                                   request->requestor_url,
                                   request->services.Pass(),
                                   request->exposed_services.Pass(),
                                   request->filter.Pass(),
                                   base::Closure());
   }
   STLDeleteElements(&queued_client_requests);
 }
 
 void ApplicationInstance::OnQuitRequestedResult(bool can_quit) {
   if (can_quit)
     return;
 
   queue_requests_ = false;
   for (auto request : queued_client_requests_) {
     CallAcceptConnection(request->originator,
                          request->requestor_url,
                          request->services.Pass(),
                          request->exposed_services.Pass(),
                          request->requested_url);
   }
   STLDeleteElements(&queued_client_requests_);
 }
 
 }  // namespace shell
 }  // namespace mojo