Check the response URL origin in BufferedDataSource to avoid mixing cross-origin responses.

media/blink/buffered_data_source.cc

 // Copyright 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 "media/blink/buffered_data_source.h"
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/location.h"
 #include "base/single_thread_task_runner.h"
 #include "media/base/media_log.h"
 #include "net/base/net_errors.h"
+#include "third_party/WebKit/public/platform/WebSecurityOrigin.h"
+#include "third_party/WebKit/public/web/WebFrame.h"
 
 using blink::WebFrame;
 
 namespace {
 
 // BufferedDataSource has an intermediate buffer, this value governs the initial
 // size of that buffer. It is set to 32KB because this is a typical read size
 // of FFmpeg.
 const int kInitialReadBufferSize = 32768;
 
@@ skipping 326 matching lines @@
 
   bool init_cb_is_null = false;
   {
     base::AutoLock auto_lock(lock_);
     init_cb_is_null = init_cb_.is_null();
   }
   if (init_cb_is_null) {
     loader_->Stop();
     return;
   }
+  response_original_url_ = loader_->response_original_url();
 
   // All responses must be successful. Resources that are assumed to be fully
   // buffered must have a known content length.
   bool success = status == BufferedResourceLoader::kOk &&
                  (!assume_fully_buffered() ||
                   loader_->instance_size() != kPositionNotSpecified);
 
   if (success) {
     total_bytes_ = loader_->instance_size();
     streaming_ =
@@ skipping 27 matching lines @@
                                    loader_->range_supported());
   }
 
   base::ResetAndReturn(&init_cb_).Run(success);
 }
 
 void BufferedDataSource::PartialReadStartCallback(
     BufferedResourceLoader::Status status) {
   DCHECK(render_task_runner_->BelongsToCurrentThread());
   DCHECK(loader_.get());
-
-  if (status == BufferedResourceLoader::kOk) {
+  if (status == BufferedResourceLoader::kOk &&
+      CheckPartialResponseURL(loader_->response_original_url())) {
     // Once the request has started successfully, we can proceed with
     // reading from it.
     ReadInternal();
     return;
   }
 
   // Stop the resource loader since we have received an error.
   loader_->Stop();
 
   // TODO(scherkus): we shouldn't have to lock to signal host(), see
   // http://crbug.com/113712 for details.
   base::AutoLock auto_lock(lock_);
   if (stop_signal_received_)
     return;
   ReadOperation::Run(read_op_.Pass(), kReadError);
 }
 
+bool BufferedDataSource::CheckPartialResponseURL(
+    const GURL& partial_response_original_url) const {
+  // If the SecurityOrigin of the frame can read content of the new response, we

[hubbe, 2015/07/06 17:22:47]

Why?
Why would we ever support redirects pointing to a new place in the middle of
playback?

Same question for the service worker thingy below.

[horo, 2015/07/07 01:06:46]

In current implementation, redirects pointing to a new place in the middle of
playback is supported.
And what I want to fix here is to prevent the data leak attack which is using
malicious header bytes and redirect pointing to target URL
(http://crbug.com/489060#c32).

If canRequest() is true, the content in the new URL is readable by the document
(eg: same origin.).
Service Worker scripts must be placed on the same origin server.
So when the response is generated in a Service Worker, it is OK to treat the
response as readable by the document.
When the new URL is readable by the document, it is OK to support the redirect.

According to https://crbug.com/74451#c8, YouTube and Vimeo are using redirect.
This patch http://codereview.chromium.org/6580014 "Make playback fail if
redirected to a different origin" was reverted because of those existing users.
Do you think we can say that there is no user who is using redirects in the
middle of playback now?

[falken, 2015/07/07 02:32:49]

To clarify: YouTube and Vimeo use redirects for the first response, but not
redirects to a new place in the middle of playback (so, we believe this patch
won't break them, unlike the patch that was reverted).

[hubbe, 2015/07/08 18:13:23]

That's what I thought too. Which I think means that this function doesn't need
the extra exceptions.

[horo, 2015/07/09 00:14:27]

I don't know the real world usage of the media elements.
Could you please let me know whether it is OK to allow same-origin redirects in
the middle of playback?

evn@ said OK. (crbug.com/505829#c15)
But if you say no, I will change to check the URL, not the origin URL.

+  // accept.
+  if (frame_->securityOrigin().canRequest(partial_response_original_url))
+    return true;
+
+  // If the response is generated in a Service Worker we accept.

[falken, 2015/07/07 02:32:49]

Please mention here something about why, i.e., the Service Worker script is
guaranteed to have the same origin as document requesting this URL, so it can
only lie to itself (is that correct?).

[horo, 2015/07/07 03:48:36]

yes.
added comments.

+  if (!partial_response_original_url.is_valid())

[falken, 2015/07/07 02:32:49]

is_empty() instead of is_valid

Sidenote: It's a bit sad this is the only way to detect a SW-generated response.

[horo, 2015/07/07 03:48:36]

Done.

+    return true;
+
+  // Otherwise we don't support mixing different origin responses. If we support
+  // this, malicious attackers can scan the bytes of other origin resources by
+  // mixing their generated bytes and the target response. See
+  // http://crbug.com/489060#c32 for details.
+  return response_original_url_.GetOrigin() ==
+         partial_response_original_url.GetOrigin();
+}
+
 void BufferedDataSource::ReadCallback(
     BufferedResourceLoader::Status status,
     int bytes_read) {
   DCHECK(render_task_runner_->BelongsToCurrentThread());
 
   // TODO(scherkus): we shouldn't have to lock to signal host(), see
   // http://crbug.com/113712 for details.
   base::AutoLock auto_lock(lock_);
   if (stop_signal_received_)
     return;
@@ skipping 116 matching lines @@
   }
 
   // If media is currently playing or the page indicated preload=auto or the
   // the server does not support the byte range request or we do not want to go
   // too far ahead of the read head, use threshold strategy to enable/disable
   // deferring when the buffer is full/depleted.
   loader_->UpdateDeferStrategy(BufferedResourceLoader::kCapacityDefer);
 }
 
 }  // namespace media