New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 334204 link

Starred by 1 user

Same-origin security issue in <video> on Android

Reported by phil...@opera.com, Jan 14 2014

Issue description

HÃ¥vard Molland at Opera has found a potential security issue with <video> on Android. To quote our internal bug report:

When playing video from <video> tag, chromium on android uses a native media player. However, once the video url is handed over to the media player, Chromium has no longer control.

To figure out same origin issues, chromium sends a info request on the video url before handing over the url. If the request is redirected to different domain, the same origin flag is set to false. This is not enough, as an attacker could simply choose to not redirect the info request, while always redirecting the native player.

The attack goes like this.
user visits evil.com, and is logged in on homevideos.com.

0. user loads the evil.com page with <video> tag, where src is pointing to evil.com
1. user starts playing video, and chromium sends the info request to evil.com.
2. evil.com receives the request, and serve a response without redirecting.
3. chromium hands over the url to the native player which sends the video request to evil.com
4. evil.com receives the request, but redirects it to homevideos.com
5. the evil.com page is now playing a cross-origin video from homevideos.com, but Blink thinks that it is playing a same-origin video from evil.com.

This attack opens up for extracting video frames cross-origin by painting the video to a canvas. When in-band subtitles are supported it will also allow extracting the times and texts of subtitles from cross-origin videos.

 

Comment 1 by phil...@opera.com, Jan 14 2014

Cc: haava...@opera.com

Comment 2 by palmer@chromium.org, Jan 14 2014

Cc: palmer@chromium.org

Comment 3 by kenrb@chromium.org, Jan 14 2014

Cc: qin...@chromium.org apiccion@chromium.org
Thanks for the report, this looks like an interesting bug.

qinmin@ and apiccion@, are either of you able to investigate this?

Comment 4 by qin...@chromium.org, Jan 14 2014

I think the best solution is to have the android MediaPlayer to support the cross origin check.
Or otherwise, we have to change the current implementation to do decoding inside chrome, rather than having a native player. That will involve a lot of code changes and may cause performance hiccups.

Comment 5 by phil...@opera.com, Jan 15 2014

As far as we've been able to tell, Android's MediaPlayer doesn't provide any way of detecting redirects, it just takes a URL and runs with it.

Long term I indeed think that the best path, for many reasons, is to use as little platform-specific code as possible, i.e. to use Android's new MediaCodec API to get accelerated decoding but nothing else of its media stack. However, that's way out of scope for this issue, we can't wait for that if there's really a security issue here.
phillipj: that is indeed a long term plan that I hope to investigate/prototype in the coming weeks


that being said ... is it correct to say we're kind of stuck here at the moment? even if there were new Android APIs introduced they won't arrive for quite a while :\

Comment 7 by kenrb@chromium.org, Jan 15 2014

Labels: Security_Impact-Beta Security_Impact-Stable Security_Severity-Medium
Status: Available

Comment 8 by qin...@chromium.org, Jan 15 2014

MediaCodec is quite buggy, especially on Samsung devices.
If we choose MediaCodec, <video> will break on a lot of ICS and JB devices
Project Member

Comment 9 by ClusterFuzz, Jan 15 2014

Labels: -Pri-2 Pri-1

Comment 10 by phil...@opera.com, Jan 16 2014

scherkus: Yeah, we're stuck with what we have for probably years to come, so the question is what to do about it. Just treating all videos as cross-origin on Android is the only thing I can come up with, and the impact of that ought to be acceptable, I think, since using canvas and Web Audio API with media elements should still be uncommon.

Comment 11 by phil...@opera.com, Jan 16 2014

Cc: sigbj...@opera.com mar...@opera.com ma...@opera.com nils...@opera.com
Labels: Cr-Blink Cr-Blink-Video
Project Member

Comment 13 by ClusterFuzz, Jan 25 2014

Labels: Owner-Triage
Owner: fischman@chromium.org
Status: Assigned
fischman: Can you please take a look or find someone else to own it.

You are auto-assigned this issue since you are the top fixer for area label 'Cr-Blink-Video'.

- Your friendly ClusterFuzz
Labels: Cr-Internals-Media
@philipj: IDK about "should still be uncommon"; when I broke <video>-to-<canvas> (with the introduction of HW video decode on windows) I heard quite a bit about it until it was fixed...

@clusterfuzz: you're cute when you try to AI.

Is it possible to replace MediaPlayer.setDataSource()'s use of a URL with a Content URI whose authority/path/id points back to Chromium code for getting at the bytes?  In other words, use chromium's HTTP stack instead of Android's by making Android go through its content/intent-resolution stack only to find chromium waiting at the other end?

(ideally, MediaPlayer.setDataSource would have a variant that accepted an implementable interface, like setDataSource(const sp<IStreamSource> &source) in https://android.googlesource.com/platform/frameworks/av/+/master/media/libmedia/IMediaPlayer.cpp but AFAICT that's not exposed to the Java SDK :()

Comment 15 by phil...@opera.com, Jan 30 2014

I didn't know about Content URIs, but that looks worth investigating. (I don't even have an Android development environment set up now, so sorry for not being so helpful.)
internal b/12573548 against the android Media team
Project Member

Comment 17 by ClusterFuzz, Feb 3 2014

Labels: Nag
fischman@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Cc: fischman@chromium.org
Owner: wolenetz@chromium.org
@wolenetz/qinmin: do you want to take a crack at implementing my suggestion from #14 above?  (assigning to wolenetz arbitrarily; feel free to hot-potato it)

Obviously, depending on a future MediaPlayer to fix this is in conflict with the Sec_Severity-Medium/P1 classification of this bug.  I'm not sure that classification is warranted, but I'm also not sure how those decisions are made.
Owner: qin...@chromium.org
@qinmin: Would you please take this one? See especially comments #14 and #18 for a potential shorter-term work-around/fix.
Project Member

Comment 20 by ClusterFuzz, Feb 12 2014

qinmin@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Sorry, I was on vacation for the past several weeks.
I don't think #14 can solve the issue without any changes to android. 
b/12573548 is probably the best solution
@qinmin: android-side changes would be many months out (and inapplicable to existing devices until/if they update).  
Why do you not think #14 can solve the issue without any changes to android?  In case it was unclear, I'm suggesting that chrome register as a ContentProvider, and feed android's media classes that require a URI one that is served by chrome, and which chrome would serve by feeding the bytes it got from the network.
If Chrome is a contentProvider, is it possible for other apps to query chrome with the same content uri to get the data that the browser is playing? isn't that causing another security concern?

re: #23: I don't know.  Maybe one-time-use GUIDs in the URI can make it secure, maybe chrome can say who it'll vend the content to, and so on.  That's why this is just an idea and not a fleshed-out design.  I was hoping you could take a crack at fleshing it out and making it work.
Yes, Chrome could refuse to expose its ContentProvider to any client other than the system.

(1) Chrome could do it dynamically by using http://developer.android.com/reference/android/content/ContentProvider.html#getCallingPackage() and making sure the Android Media Player is truly the calling package.

Or,

(2) Chrome could, for example, declare a new Permission, declare it to be signatureOrSystem, and then require that clients hold that permission.

http://developer.android.com/guide/topics/manifest/provider-element.html#prmsn

I think (1) would probably work best, because it doesn't require cooperation from Media Player.
for (1), what if another app also creates an android MediaPlayer and passes the same content URI to the MediaPlayer? The app can then read back the surface texture whenever a frame is decoded by the player
Project Member

Comment 27 by ClusterFuzz, Feb 27 2014

qinmin@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Labels: M-32
Labels: -M-32 M-33
Project Member

Comment 30 by ClusterFuzz, Mar 7 2014

qinmin@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Project Member

Comment 31 by ClusterFuzz, Mar 16 2014

qinmin@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Project Member

Comment 32 by ClusterFuzz, Mar 24 2014

qinmin@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Project Member

Comment 33 by ClusterFuzz, Mar 31 2014

Labels: -M-33 M-34
Project Member

Comment 34 by ClusterFuzz, Apr 2 2014

qinmin@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Project Member

Comment 35 by ClusterFuzz, Apr 10 2014

qinmin@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Owner: scherkus@chromium.org
scherkus: Can you please take this, or find someone to own it? This security vulnerability has languished for too long.
Project Member

Comment 37 by ClusterFuzz, Apr 19 2014

scherkus@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
qinmin: think you have any cycles for tackling this in Q2?
From the discussion #22-#26, there seems no clean way to fix this without any android support.
b/12573548 provide a better tracking for this.

How severe is the security threat? can we disable canvas painting for video?


Comment 40 by phil...@opera.com, Apr 22 2014

I don't know how to rank security issues, but ability to extract frames cross-origin seems pretty bad. My thinking from the start was that treating all resources that pass through android.media.MediaPlayer as cross-origin would be the sanest way to fix this. I'm sure that will break something, but I can't see a better option.
Cc: lajos@google.com
Labels: -Security_Severity-Medium -Pri-1 -M-34 Security_Severity-High Pri-0 M-35
According to http://www.chromium.org/developers/severity-guidelines:

"""
High Severity

Allows an attacker to read or modify confidential data belonging to other web sites. """

So, this should probably have been called a High. Since it is so old, I am bumping it up to Pri-0. Unfortunately, we probably also won't get a fix until M-35. But let's please not let it slip any further than that.

I agree with Philip's notion that we might just have to treat resources that go through Android MediaPlayer as cross-origin.
ok, i can mark everything as cross-origin.
The impact is that all video canvas painting and some webgl will fail to run.
Do web sites commonly paint video onto a canvas? Most use <video> or Flash, right?

The WebGL issue seems more concerning. How do web sites run WebGL now? Don't they source GL libraries into their own origins? (Sorry, I am completely ignorant of this area.)
video canvas painting is used by a lot of WebGL pages, that's why some of them will fail.


Project Member

Comment 46 by ClusterFuzz, Apr 22 2014

Labels: ReleaseBlock-Stable
Labels: -Security_Severity-High Security_Severity-Medium
It's reasonable to bump priority on an ignored bug, but the severity does not change. High severity would be an unmitigated origin bypass, which this is not. Yes, the allows leaking information between origins, but it is a limited type of information (video frames).
Cc: dutton@chromium.org
Project Member

Comment 49 by bugdroid1@chromium.org, Apr 27 2014

Labels: merge-merged-git-svn
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f689da10139d76dd3fb8a0e92d5732324cb2d90c

commit f689da10139d76dd3fb8a0e92d5732324cb2d90c
Author: qinmin@chromium.org <qinmin@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Sun Apr 27 00:17:04 2014 +0000

Always mark cross-origin as true for regular media urls

We cannot guarantee that urls will not be redirected when android mediaplayer requests video streams.
Always treat media urls as cross-origin for now.

BUG= 334204 

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@266396 0039d316-1c4b-4281-b951-d872f2087c98


Project Member

Comment 50 by bugdroid1@chromium.org, Apr 27 2014

------------------------------------------------------------------
r266396 | qinmin@chromium.org | 2014-04-27T00:17:04.880780Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/media/android/webmediaplayer_android.cc?r1=266396&r2=266395&pathrev=266396
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/media/android/media_info_loader.h?r1=266396&r2=266395&pathrev=266396

Always mark cross-origin as true for regular media urls

We cannot guarantee that urls will not be redirected when android mediaplayer requests video streams.
Always treat media urls as cross-origin for now.

BUG= 334204 

Review URL: https://codereview.chromium.org/247573004
-----------------------------------------------------------------
Labels: Merge-Requested

Comment 52 by kareng@google.com, Apr 28 2014

Labels: -Merge-Requested Merge-Approved
pls confirm on tot before you merge.
Cc: kbr@chromium.org

Comment 54 by kbr@chromium.org, Apr 28 2014

Cc: siev...@chromium.org skyos...@chromium.org zmo@chromium.org alokp@chromium.org bajones@chromium.org
Labels: Cr-Blink-WebGL Cr-Internals-GPU-Testing
I only recently became aware of this issue. How realistic and severe do we really think this attack scenario is? An attacker stealing pixels from confidential images (bank statements, credit card statements, etc.) is one thing, but pixels from authenticated or private videos?

The most visible impact of this change is that it breaks the WebGL conformance suite, so that sites uploading videos to WebGL no longer work on Chrome on Android. I consider this to be a severe regression. It has also turned the Android GPU bot red. http://build.chromium.org/p/chromium.gpu/builders/Android%20Debug%20%28Nexus%207%29?numbuilds=200

I would like to request that this fix and the marked security severity be reconsidered, especially since there is no roadmap for fixing it completely in Android.

alright, I will hold off the merge until security folks confirm this is a serious threat and something we should really address.

Comment 56 by kbr@chromium.org, Apr 28 2014

Blockedon: chromium:367962
Project Member

Comment 57 by bugdroid1@chromium.org, Apr 29 2014

Labels: -Merge-Approved
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/207c0196c0d8c4d82c1ab15adde3ad6ffce14e47

commit 207c0196c0d8c4d82c1ab15adde3ad6ffce14e47
Author: alokp@chromium.org <alokp@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Tue Apr 29 00:20:48 2014 +0000

Disable some webgl conformance tests due to a security issue on video canvas painting

Check  http://crbug.com/334204  for details

BUG= 334204 , 367962 
R=alokp@chromium.org, kbr@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@266716 0039d316-1c4b-4281-b951-d872f2087c98


Project Member

Comment 58 by bugdroid1@chromium.org, Apr 29 2014

------------------------------------------------------------------
r266716 | alokp@chromium.org | 2014-04-29T00:20:48.763823Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/test/gpu/gpu_tests/webgl_conformance_expectations.py?r1=266716&r2=266715&pathrev=266716

Disable some webgl conformance tests due to a security issue on video canvas painting

Check  http://crbug.com/334204  for details

BUG= 334204 , 367962 
R=alokp@chromium.org, kbr@chromium.org

Review URL: https://codereview.chromium.org/251113005
-----------------------------------------------------------------

Comment 59 by sigbj...@opera.com, Apr 29 2014

A realistic attack target would be an internal company presentation with confidential information (quite common many places). Personally (working in the security group), I sometimes get sent links to private videos showing how an exploit works. This bug means that if those URLs can be guessed/retrieved by an attacker, I need to disable <video> to browse safely on Android, otherwise I risk them leaking without my consent.

Once the next big thing takes off, like an instamov successor to instagram, this bug will hit Android hard. If you are logged in, any site you visit can see all your private videos.

If videos can be played inline/invisibly, then videos can leak without user knowledge, and an attacker can brute force URLs to find the videos.
Blocking: chromium:368418
alokp@ - any other changes required here? If not, please mark as fixed.

Comment 62 by k...@google.com, May 2 2014

I think we're waiting to hear if this needs a merge or not for 35 before closing.
Pinged jschuh@ - he is going to discuss with palmer@ to see if this should wait for M36 or merge into M35 from a security POV.
Labels: -M-35 M-36
Given the compatibility risk we're comfortable waiting for this to roll in normally.
Status: Fixed
Forgot the fixed label.
Labels: Release-0-M36
Labels: reward-ineligible
Project Member

Comment 68 by ClusterFuzz, May 13 2014

Labels: -Restrict-View-SecurityTeam
Labels: CVE-2014-3161

Comment 70 by kbr@chromium.org, Jul 19 2014

Blocking: chromium:358198
Project Member

Comment 71 by ClusterFuzz, Aug 14 2014

Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.
Project Member

Comment 72 by ClusterFuzz, Feb 2 2016

Labels: -Security_Impact-Beta
Components: -Blink>Video Blink>Media>Video
Renaming Blink>Video to Blink>Media>Video for better characterization
Project Member

Comment 74 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 75 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
Labels: CVE_description-submitted

Sign in to add a comment