RDH: Block a compromised renderer from reusing request ids

content/browser/loader/resource_dispatcher_host_impl.cc

 // Copyright (c) 2012 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.
 
 // See http://dev.chromium.org/developers/design-documents/multi-process-resource-loading
 
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
 
 #include <stddef.h>
 #include <algorithm>
@@ skipping 1034 matching lines @@
                                 info->GetResourceType());
   }
 
   if (delegate_)
     delegate_->RequestComplete(loader->request());
 
   // Destroy the ResourceLoader.
   RemovePendingRequest(info->GetChildID(), info->GetRequestID());
 }
 
+void ResourceDispatcherHostImpl::LoaderDestroyed(ResourceLoader* loader) {
+  request_ids_in_use_.erase(loader->GetRequestInfo()->GetGlobalRequestID());
+}
+
 void ResourceDispatcherHostImpl::OnInit() {
   scheduler_.reset(new ResourceScheduler);
 }
 
 void ResourceDispatcherHostImpl::OnShutdown() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   is_shutdown_ = true;
   pending_loaders_.clear();
 
@@ skipping 98 matching lines @@
 // If sync_result is non-null, then a SyncLoad reply will be generated, else
 // a normal asynchronous set of response messages will be generated.
 void ResourceDispatcherHostImpl::OnSyncLoad(
     int request_id,
     const ResourceHostMsg_Request& request_data,
     IPC::Message* sync_result) {
   BeginRequest(request_id, request_data, sync_result,
                sync_result->routing_id());
 }
 
 void ResourceDispatcherHostImpl::UpdateRequestForTransfer(

[mmenke, 2016/01/19 20:30:31]

BUG:  This changes the id of a request, so needs to update the set as well.  I'm
not a huge fan of the information duplicating approach because it inevitably
leads to bug like this.  Wonder if we can come up with something better...

     int child_id,
     int route_id,
     int request_id,
     const ResourceHostMsg_Request& request_data,
     const linked_ptr<ResourceLoader>& loader) {
   ResourceRequestInfoImpl* info = loader->GetRequestInfo();
   GlobalRoutingID old_routing_id(
       request_data.transferred_request_child_id, info->GetRouteID());
   GlobalRequestID old_request_id(request_data.transferred_request_child_id,
                                  request_data.transferred_request_request_id);
@@ skipping 63 matching lines @@
 }
 
 void ResourceDispatcherHostImpl::BeginRequest(
     int request_id,
     const ResourceHostMsg_Request& request_data,
     IPC::Message* sync_result,  // only valid for sync
     int route_id) {
   int process_type = filter_->process_type();
   int child_id = filter_->child_id();
 
+  // Reject request id that's currently in use.
+  if (request_ids_in_use_.count(GlobalRequestID(child_id, request_id))) {
+    bad_message::ReceivedBadMessage(
+        filter_,
+        bad_message::RDH_INVALID_REQUEST_ID);
+    return;
+  }
+
   // PlzNavigate: reject invalid renderer main resource request.
   if (IsBrowserSideNavigationEnabled() &&
       IsResourceTypeFrame(request_data.resource_type) &&
       !request_data.url.SchemeIs(url::kBlobScheme)) {
     bad_message::ReceivedBadMessage(filter_, bad_message::RDH_INVALID_URL);
     return;
   }
 
   // Reject invalid priority.
   if (request_data.priority < net::MINIMUM_PRIORITY ||
@@ skipping 1017 matching lines @@
     IncrementOutstandingRequestsMemory(-1, *info);
 
     // A ResourceHandler must not outlive its associated URLRequest.
     handler.reset();
     return;
   }
 
   linked_ptr<ResourceLoader> loader(
       new ResourceLoader(std::move(request), std::move(handler), this));
 
+  request_ids_in_use_.insert(info->GetGlobalRequestID());
+
   GlobalRoutingID id(info->GetGlobalRoutingID());
   BlockedLoadersMap::const_iterator iter = blocked_loaders_map_.find(id);
   if (iter != blocked_loaders_map_.end()) {
     // The request should be blocked.
     iter->second->push_back(loader);
     return;
   }
 
   StartLoading(info, loader);
 }
@@ skipping 245 matching lines @@
     load_flags |= net::LOAD_PREFETCH;
   }
 
   if (is_sync_load)
     load_flags |= net::LOAD_IGNORE_LIMITS;
 
   return load_flags;
 }
 
 }  // namespace content