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 0 users
Status: Fixed
Owner:
Closed: Aug 2014
Cc:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security



Sign in to add a comment
Stack-buffer-overflow in avpriv_aac_parse_header
Project Member Reported by clusterf...@chromium.org, Aug 20 2014 Back to list
Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5768278820519936

Fuzzer: Inferno_flicker
Job Type: Linux_asan_chrome_media

Crash Type: Stack-buffer-overflow READ 4
Crash Address: 0x7f49597c09aa
Crash State:
  avpriv_aac_parse_header
  aac_sync
  ff_aac_ac3_parse
  

Minimized Testcase (123.47 Kb): https://cluster-fuzz.appspot.com/download/AMIfv940erZA0ODRl7BYyUYwNqv1rt6JpIG-RkAja2h-4HhZvzeDKNPuluVzGETSdR2BqmbJTMoQlRqNEZZhXQxxHoFzuVl5h1GyjkHt5NS_X1ZDXDciBsAJj2nG7I_4mVR-QK1i9rde_pdTEOUtkbfai0xDhboPjiYSSTUiTwDp4fGGfqWiIaA

Filer: inferno
 
Cc: scherkus@chromium.org
Owner: dalecur...@chromium.org
Status: Assigned
Project Member Comment 2 by clusterf...@chromium.org, Aug 20 2014
Labels: Pri-1
Labels: Security_Impact-Stable
assuming stable, unless Dale can confirm.
Labels: M-38
Cc: dalecur...@chromium.org
Owner: wolenetz@chromium.org
Probably M38 only since that just rolled.
Labels: -Security_Impact-Stable Security_Impact-Beta
Status: Started
Investigating...
I have a local repro. I'm not sure the roll caused this or it predates the M38 ffmpeg roll. Digging deeper...
It looks like aac_sync's usage of init_get_bits() provides an insufficiently padded bit buffer to read from.  I'm unsure if this is the only problem here.  Still digging.
if it impacts stable, feel free to flip the impact label back (see c#6).
I have a change to our downstream ToT chromium FFmpeg out for review that fixes this at https://gerrit.chromium.org/gerrit/#/c/71288/1

I'll also look to see if this needs merging back to stable, too.
ToT fix is in CR @ https://codereview.chromium.org/499643003/
I will let ToT bake, then request merge to M38. There are "clang warning removal" and a README.chromium ffmpeg changes on ToT between M38 branch and this fix; including those changes in merge to M38 shouldn't be a problem.
Project Member Comment 14 by bugdroid1@chromium.org, Aug 22 2014
------------------------------------------------------------------
r291545 | wolenetz@chromium.org | 2014-08-22T22:59:32.584004Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/DEPS?r1=291545&r2=291544&pathrev=291545

Roll FFmpeg DEPS

Includes cherry-pick to fix aac/ac3 parser bitstream buffer size

BUG= 405416 
R=dalecurtis@chromium.org
TEST=media_unittests all pass with proprietary codecs with asan

Review URL: https://codereview.chromium.org/499643003
-----------------------------------------------------------------
Project Member Comment 15 by bugdroid1@chromium.org, Aug 22 2014
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7f59a2dc18e21b332b7aa7148e532d16312a36de

commit 7f59a2dc18e21b332b7aa7148e532d16312a36de
Author: wolenetz@chromium.org <wolenetz@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Fri Aug 22 22:59:32 2014

Roll FFmpeg DEPS

Includes cherry-pick to fix aac/ac3 parser bitstream buffer size

BUG= 405416 
R=dalecurtis@chromium.org
TEST=media_unittests all pass with proprietary codecs with asan

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

Cr-Commit-Position: refs/heads/master@{#291545}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@291545 0039d316-1c4b-4281-b951-d872f2087c98


Status: Fixed
Project Member Comment 17 by clusterf...@chromium.org, Aug 23 2014
Labels: -Restrict-View-SecurityTeam Merge-Triage Restrict-View-SecurityNotify
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
Project Member Comment 18 by clusterf...@chromium.org, Aug 24 2014
ClusterFuzz has detected this issue as fixed in latest custom build.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5768278820519936

Fuzzer: Inferno_flicker
Job Type: Linux_asan_chrome_media

Crash Type: Stack-buffer-overflow READ 4
Crash Address: 0x7f49597c09aa
Crash State:
  avpriv_aac_parse_header
  aac_sync
  ff_aac_ac3_parse
  

Minimized Testcase (123.47 Kb): https://cluster-fuzz.appspot.com/download/AMIfv940erZA0ODRl7BYyUYwNqv1rt6JpIG-RkAja2h-4HhZvzeDKNPuluVzGETSdR2BqmbJTMoQlRqNEZZhXQxxHoFzuVl5h1GyjkHt5NS_X1ZDXDciBsAJj2nG7I_4mVR-QK1i9rde_pdTEOUtkbfai0xDhboPjiYSSTUiTwDp4fGGfqWiIaA

If you suspect that the result above is incorrect, try re-doing that job on the testcase report page.

Labels: Merge-Requested
Status: Started
This has baked on canary with no issues observed so far. Requesting merge to beta (m38).
Status: Fixed
Please keep bug in status fixed, merge is tracked via merge labels.
Labels: -Security_Impact-Beta Security_Impact-Stable
Impacts stable, too:
Chromium ffmpeg commit 5c3de8094903dd2162232b8f1be916c46acdd8f5 repros issue as I reported upstream using clang-asan toolchain, steps in our downstream to repro:
0. ensure chromium yasm and clang are in path
1. [update libavcodec/old_codec_ids.h to insert the deprecated SNOW codec ID in the sequence where it's in avcodec.h (this enables execution of ffmpeg standalone binary to not hit assert on mismatching codec IDs).
2. ./configure --samples=fate-suite/ --toolchain=clang-asan && make fate-rsync && make
3a. ./ffmpeg -i fate-suite/aac/ct_faac-adts.aac tmp.pcm
->(boom)
similar for ac3, though we don't use that in downstream; similar fix is included in cherry-pick from upstream for ToT, and intended for M38 and M37 on merge-approval, respectively:
3b. ./ffmpeg -i fate-suite/ac3/millers_crossing_4.0.ac3 /dev/null
->(boom)
Labels: -Merge-Triage M-37
inferno@: w.r.t. #22, should we leave it in M38 until merge approved (and merge-merged) for m38, and then change it to M37 and rinse/repeat merge request/approval for M37? Perhaps process has changed; I'm confused.
We can have multiple milestones per bug. m38 rm will approve it for m38 first, then you can remove m38 and re-request for m37 by putting merge-requested.
@#24: thank you for explaining. sgtm.
Labels: -Merge-Requested Merge-Approved
Cc: wolenetz@chromium.org
Owner: dalecur...@chromium.org
fyi - I'll be OoO through Monday.

Merge of this to M38 is still pending clarification of the steps necessary (we thought we had figured out the usual drover steps should work, but there is now concern about exactly which DEPS file should get updated -- we think drover might update the wrong one). dalecurtis@, matthewyuan@ and I are working this out in email.

Dale: can you take care of follow-up on this for M38 merge please?
Project Member Comment 28 by bugdroid1@chromium.org, Aug 29 2014
Labels: -Merge-Approved merge-merged-2125
The following revision refers to this bug:
  http://goto.ext.google.com/viewvc/chrome-internal?view=rev&revision=59871

------------------------------------------------------------------
r59871 | dalecurtis@google.com | 2014-08-29T00:32:40.941805Z

-----------------------------------------------------------------
Labels: -M-38 Merge-Requested
Per #21, this impacts M37 too. Is this severe enough to warrant merge to M37?
Cc: -wolenetz@chromium.org
Owner: wolenetz@chromium.org
Labels: -M-37 -Merge-Requested Release-0-M38
Skipping m37 merge based on medium severity.
Project Member Comment 32 by clusterf...@chromium.org, Dec 2 2014
Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.
Project Member Comment 33 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 34 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