Fix UAF in new Mojo EDK.

mojo/edk/system/routed_raw_channel.cc

Index: mojo/edk/system/routed_raw_channel.cc
diff --git a/mojo/edk/system/routed_raw_channel.cc b/mojo/edk/system/routed_raw_channel.cc
index cd57693cbdcd401b3678b09f4595890b593c2ab0..c3742491742c16f7f574adf2ca0457cb3a56cb27 100644
--- a/mojo/edk/system/routed_raw_channel.cc
+++ b/mojo/edk/system/routed_raw_channel.cc
@@ -59,7 +59,9 @@ void RoutedRawChannel::AddRoute(uint64_t route_id,
 
 void RoutedRawChannel::RemoveRoute(uint64_t route_id) {
   DCHECK(internal::g_io_thread_task_runner->RunsTasksOnCurrentThread());
-  CHECK(routes_.find(route_id) != routes_.end());
+  // We don't check that routes_ contains route_id because it's possible for it
+  // to not have been added yet (i.e. it's waiting to be added later down the
+  // call stack).
   routes_.erase(route_id);
 
   // Only send a message to the other side to close the route if we hadn't
@@ -132,24 +134,21 @@ void RoutedRawChannel::OnReadMessage(
 
 void RoutedRawChannel::OnError(Error error) {
   DCHECK(internal::g_io_thread_task_runner->RunsTasksOnCurrentThread());
-
-  // This needs to match non-multiplexed MessagePipeDispatcher's destruction of
-  // the channel only when read errors occur.
-  if (error != ERROR_WRITE || routes_.empty()) {
-    channel_->Shutdown();
-    channel_ = nullptr;
+
+  // Note: we must ensure we don't call RawChannel::Shutdown until after we've
+  // called OnError on each route's delegate.
+  for (auto it = routes_.begin(); it != routes_.end();) {

[Fady Samuel, 2015/12/17 05:35:02]

I don't really understand this loop Is the intention to skip the first time?
what if the it++ == routes_.end()? Wouldn't following second fail?

[Fady Samuel, 2015/12/17 05:38:33]

nm, it's late and I'm not thinking straight. It's still weird here. Why do you
need cur_it?

[jam, 2015/12/17 05:45:16]

improved comment

+    // Handle the delegate calling RemoveRoute in this call.
+    auto cur_it = it++;
+    cur_it->second->OnError(error);
   }
 
   if (routes_.empty()) {
+    channel_->Shutdown();
+    channel_ = nullptr;
     delete this;
     return;
   }
-
-  for (auto it = routes_.begin(); it != routes_.end();) {
-    // Handle the delegate calling RemoveRoute in this call.
-    auto cur_it = it++;
-    cur_it->second->OnError(error);
-  }
 }
 
 }  // namespace edk