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

Issue metadata

Status: Fixed
Owner:
Email to this user bounced
Closed: Oct 2011
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug-Security

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment

OOB read in OGV at unpack_vlcs

Reported by aohe...@gmail.com, Oct 15 2011

Issue description

VULNERABILITY DETAILS
ASAN reports a global-buffer-overflow when the attached OGV video is played. I haven't yet had a look if it seems to have security impact, but filing conservatively as a security bug.

VERSION
Chrome Version: Chromium 16.0.910.0 (Developer Build 105656)
Operating System: Linux (Debian 6.0.3, x84_64)

REPRODUCTION CASE
 $ chrome oobr.ogv

FOR CRASHES, PLEASE INCLUDE THE FOLLOWING ADDITIONAL INFORMATION
Type of crash: tab
Crash State: 

Program received signal SIGILL, Illegal instruction.
[Switching to Thread 0x7fffbaa2a700 (LWP 5305)]
unpack_vlcs (s=<optimized out>, gb=Unhandled dwarf expression opcode 0x0
)
    at third_party/ffmpeg/patched-ffmpeg/libavcodec/vp3.c:899
899                     eob_run = eob_run_base[token];
(gdb) list
894         while (coeff_i < num_coeffs && get_bits_left(gb) > 0) {
895                 /* decode a VLC into a token */
896                 token = get_vlc2(gb, vlc_table, 11, 3);
897                 /* use the token to get a zero run, a coefficient, and an eob run */
898                 if (token <= 6) {
899                     eob_run = eob_run_base[token];
900                     if (eob_run_get_bits[token])
901                         eob_run += get_bits(gb, eob_run_get_bits[token]);
902
903                     // record only the number of blocks ended in this plane,
(gdb) bt 5
#0  unpack_vlcs (s=<optimized out>, gb=Unhandled dwarf expression opcode 0x0
)
    at third_party/ffmpeg/patched-ffmpeg/libavcodec/vp3.c:899
#1  0x00007fffdd6d1d6c in unpack_dct_coeffs (s=<optimized out>, 
    gb=<optimized out>)
    at third_party/ffmpeg/patched-ffmpeg/libavcodec/vp3.c:1042
#2  vp3_decode_frame (avctx=<optimized out>, data=<optimized out>, 
    data_size=<optimized out>, avpkt=<optimized out>)
    at third_party/ffmpeg/patched-ffmpeg/libavcodec/vp3.c:1952
#3  0x00007fffdd6a13f2 in avcodec_decode_video2 (avctx=<optimized out>, 
    picture=<optimized out>, got_picture_ptr=<optimized out>, 
    avpkt=<optimized out>)
    at third_party/ffmpeg/patched-ffmpeg/libavcodec/utils.c:769
#4  0x00007ffff5cd8ae7 in media::FFmpegVideoDecodeEngine::DecodeFrame (
    this=<optimized out>, buffer=DWARF-2 expression error: DW_OP_reg operations must be used either alone or in conjuction with DW_OP_piece or DW_OP_bit_piece.
)
    at media/video/ffmpeg_video_decode_engine.cc:181
(More stack frames follow...)
==19248== ERROR: AddressSanitizer global-buffer-overflow on address 0x7f94dbc029dc at pc 0x7f94dbaf52fc bp 0x7f94d61d5d9c sp 0x7f94d61d53c0
READ of size 4 at 0x7f94dbc029dc thread T7
    #0 0x7f94dbaf52fc (/home/aki/chromium/src/out/Release/libffmpegsumo.so+0x2b22fc)
    #1 0x7f94dbaebd6c (/home/aki/chromium/src/out/Release/libffmpegsumo.so+0x2a8d6c)
    #2 0x7f94dbabb3f2 (/home/aki/chromium/src/out/Release/libffmpegsumo.so+0x2783f2)
    #3 0x7f94f1263ae7 (/home/aki/chromium/src/out/Release/chrome+0x77abae7)
    #4 0x7f94f126330e (/home/aki/chromium/src/out/Release/chrome+0x77ab30e)
    #5 0x7f94f12537cf (/home/aki/chromium/src/out/Release/chrome+0x779b7cf)
    #6 0x7f94f12564dc (/home/aki/chromium/src/out/Release/chrome+0x779e4dc)
    #7 0x7f94ebaad6d7 (/home/aki/chromium/src/out/Release/chrome+0x1ff56d7)
    #8 0x7f94ebaade89 (/home/aki/chromium/src/out/Release/chrome+0x1ff5e89)
    #9 0x7f94ebaaf398 (/home/aki/chromium/src/out/Release/chrome+0x1ff7398)
    #10 0x7f94ebab968a (/home/aki/chromium/src/out/Release/chrome+0x200168a)
    #11 0x7f94ebaac1d9 (/home/aki/chromium/src/out/Release/chrome+0x1ff41d9)
    #12 0x7f94ebaaa3a9 (/home/aki/chromium/src/out/Release/chrome+0x1ff23a9)
    #13 0x7f94ebb26928 (/home/aki/chromium/src/out/Release/chrome+0x206e928)
    #14 0x7f94ebb255cc (/home/aki/chromium/src/out/Release/chrome+0x206d5cc)
    #15 0x7f94f13266d5 (/home/aki/chromium/src/out/Release/chrome+0x786e6d5)
    #16 0x7f94e5d708ba (/lib/libpthread-2.11.2.so+0x68ba)
    #17 0x7f94e3ef702d (/lib/libc-2.11.2.so+0xcf02d)
0x7f94dbc029dc is located 49 bytes to the right of global variable '.str18' (0x7f94dbc02980) of size 43
  '.str18' is ascii string 'Invalid number of coefficents at level %d
'
0x7f94dbc029dc is located 4 bytes to the left of global variable 'eob_run_base' (0x7f94dbc029e0) of size 28
  'eob_run_base' is ascii string '01'

 
oobr.ogv
96.5 KB Download

Comment 1 by kcc@chromium.org, Oct 15 2011

Cc: infe...@chromium.org
Labels: Stability-AddressSanitizer
 aohelin@, just FYI: you can use the script third_party/asan/scripts/asan_symbolize.py to symbolize the asan stack traces. 

In this bug it looks like we are reading minus-one's element (token == 1):
eob_run = eob_run_base[token];

Comment 2 by aohe...@gmail.com, Oct 15 2011

@kcc: Thanks, I'll use that in the future.

I think all cases of this so far have read -1, which makes this sound less like a security bug.
Cc: scherkus@chromium.org rbultje@google.com ihf@chromium.org
@ihf, @rbultje, either of you know anything about the ffmpeg Theora video codec? :) (This probably isn't a regression, so the urgency is lower; we don't necessarily need it for M15)

Comment 4 by rbultje@google.com, Oct 16 2011

Not specifically, but I can look...

Comment 5 by rbultje@google.com, Oct 16 2011

I think we can simply change the token <= 6 check to (unsigned) token <= 6U, so that it catches the -1. It's an invalid bitstream either way so we just want to error out at that point (similar to if token were >6), and changing that check does exactly that.
Labels: -Pri-0 Pri-1 SecSeverity-Medium Mstone-16
Owner: rbultje@google.com
Status: Assigned
If negatives are generally undesired for "token", we could also change the variable type to be unsigned.

@aohelin: I presume this is an old bug that affects Chrome 15 beta too?

Comment 7 by aohe...@gmail.com, Oct 19 2011

@scarybeasts: I don't know but suspect so. This doesn't crash because the read is valid, just past the object, so not sure until my beta pull&build is done.

Comment 8 by aohe...@gmail.com, Oct 20 2011

@scarybeasts: This affects my build of r104978, which was the revision of google-chrome-beta yesterday. I used gclient sync --revision src@... because I didn't find a way yet to ask gclient to follow beta or stable.
Owner: cevans@chromium.org
Hey Ronald, I can productionize up this change and land it.

Comment 10 by rbultje@google.com, Oct 21 2011

See attached. There's tons of memleaks and I'm not quite sure how to fix them, the decoder looks a little orphaned. Since those are not crashers, I think we can safely fix them later.
0001-vp3-fix-double-free-and-invalid-read.patch
2.7 KB View Download
Yeah, we're not worried about memleaks upon invalid streams. Not a security concern.
You want me to pull the patch into Chrome or had you already started?

Comment 12 by rbultje@google.com, Oct 23 2011

Hi,

attached patch fixes all memleaks that I could find in the VP3 decoder, plus a potential infloop that I saw while testing on x86-32 (I normally develop on x86-64 and there it didn't happen; no idea why). The sample in this bug report is a good trigger. :-).

Ronald
0001-vp3-fix-a-series-of-memleaks-and-potential-infloops-.patch
5.2 KB View Download

Comment 13 by laforge@google.com, Oct 24 2011

Labels: -Mstone-16 MovedFrom-16 Mstone-17
Labels: SecImpacts-Stable SecImpacts-Beta
Owner: rbultje@google.com
Ronald has a patch up for review. Assigning to him. It's not too serious an issue so letting it roll into M17 is fine.
Project Member

Comment 15 by bugdroid1@chromium.org, Oct 27 2011

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=107489

------------------------------------------------------------------------
r107489 | scherkus@chromium.org | Wed Oct 26 17:38:14 PDT 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/deps/third_party/ffmpeg/patches/to_upstream/42_vp8_fix_segmentation_maps.patch?r1=107489&r2=107488&pathrev=107489
 M http://src.chromium.org/viewvc/chrome/trunk/deps/third_party/ffmpeg/source/patched-ffmpeg/libavcodec/vp3.c?r1=107489&r2=107488&pathrev=107489
 A http://src.chromium.org/viewvc/chrome/trunk/deps/third_party/ffmpeg/patches/to_upstream/45_mkv_fix_segmap_cache_overflow.patch?r1=107489&r2=107488&pathrev=107489
 A http://src.chromium.org/viewvc/chrome/trunk/deps/third_party/ffmpeg/patches/to_upstream/47_vp3_fix_infloop_and_memleak.patch?r1=107489&r2=107488&pathrev=107489
 M http://src.chromium.org/viewvc/chrome/trunk/deps/third_party/ffmpeg/patches/to_upstream/39_VP8_fix_oob_read_writes.patch?r1=107489&r2=107488&pathrev=107489
 M http://src.chromium.org/viewvc/chrome/trunk/deps/third_party/ffmpeg/source/patched-ffmpeg/libavcodec/vp8.c?r1=107489&r2=107488&pathrev=107489
 A http://src.chromium.org/viewvc/chrome/trunk/deps/third_party/ffmpeg/patches/to_upstream/46_vp3_fix_double_free_invalid_read.patch?r1=107489&r2=107488&pathrev=107489
 M http://src.chromium.org/viewvc/chrome/trunk/deps/third_party/ffmpeg/README.chromium?r1=107489&r2=107488&pathrev=107489
 M http://src.chromium.org/viewvc/chrome/trunk/deps/third_party/ffmpeg/patches/ugly/41_matroska_cluster_incremental.patch?r1=107489&r2=107488&pathrev=107489
 M http://src.chromium.org/viewvc/chrome/trunk/deps/third_party/ffmpeg/patches/README?r1=107489&r2=107488&pathrev=107489

Apply patches from 101172, 100465.

Update some patches so they apply with make_src_tree.sh.

Patch by rbultje@chromium.org:
http://codereview.chromium.org/8392015/

BUG= 101172 , 100465 
------------------------------------------------------------------------
Project Member

Comment 17 by bugdroid1@chromium.org, Oct 27 2011

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=107508

------------------------------------------------------------------------
r107508 | scherkus@chromium.org | Wed Oct 26 18:41:02 PDT 2011

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

Rolling FFmpeg to r107500.

BUG= 101172 , 100465 
------------------------------------------------------------------------
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Status: FixUnreleased
Labels: -SecSeverity-Medium -MovedFrom-16 -Mstone-17 SecSeverity-High Mstone-15 Merge-Approved
Bumping up severity due to the double-free aspect.
Also, might as well fix it in M15 with the other ffmpeg fixes.
Labels: reward-topanel
Labels: -reward-topanel reward-500 reward-unpaid
Thanks for the report, Aki! There was a double-free hiding behind the relatively harmless OOB read, so therefore a $500 Chromium Security Reward!

----
Boilerplate text:
Please do NOT publicly disclose details until a fix has been released to all our
users. Early public disclosure may cancel the provisional reward.
Also, please be considerate about disclosure when the bug affects a core library
that may be used by other products.
Please do NOT share this information with third parties who are not directly
involved in fixing the bug. Doing so may cancel the provisional reward.
Please be honest if you have already disclosed anything publicly or to third parties.
----
Labels: -Merge-Approved Merge-Merged
DEPS rolled on M15 branch @19346
DEPS rolled on M16 branch @19347

Comment 23 by aohe...@gmail.com, Nov 5 2011

@scarybeasts thanks for spotting the double-free :)
Labels: CVE-2011-3892
Labels: -reward-unpaid
Payment in system.

Comment 26 by cdn@chromium.org, May 15 2012

Status: Fixed
Marking old security bugs Fixed..
Project Member

Comment 27 by bugdroid1@chromium.org, Oct 13 2012

Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Project Member

Comment 28 by bugdroid1@chromium.org, Nov 14 2012

The following revision refers to this bug:
    http://goto.ext.google.com/viewvc/chrome-internal?view=rev&revision=19010

------------------------------------------------------------------------
r19010 | scherkus@google.com | 2011-10-27T01:11:20.629115Z

------------------------------------------------------------------------
Project Member

Comment 29 by bugdroid1@chromium.org, Nov 14 2012

The following revision refers to this bug:
    http://goto.ext.google.com/viewvc/chrome-internal?view=rev&revision=19011

------------------------------------------------------------------------
r19011 | scherkus@google.com | 2011-10-27T01:42:10.797392Z

------------------------------------------------------------------------
Project Member

Comment 30 by bugdroid1@chromium.org, Mar 10 2013

Labels: -Type-Security -Stability-AddressSanitizer -SecSeverity-High -Mstone-15 -SecImpacts-Stable -SecImpacts-Beta Security-Impact-Beta Security-Severity-High Security-Impact-Stable M-15 Type-Bug-Security Performance-Memory-AddressSanitizer
Project Member

Comment 31 by bugdroid1@chromium.org, Mar 11 2013

Labels: -Area-Undefined
Project Member

Comment 32 by bugdroid1@chromium.org, Mar 13 2013

Labels: Restrict-View-EditIssue
Project Member

Comment 33 by bugdroid1@chromium.org, Mar 13 2013

Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Labels: -Restrict-View-SecurityNotify -Restrict-View-EditIssue
Project Member

Comment 35 by bugdroid1@chromium.org, Mar 21 2013

Labels: -Security-Severity-High Security_Severity-High
Project Member

Comment 36 by bugdroid1@chromium.org, Mar 21 2013

Labels: -Security-Impact-Stable Security_Impact-Stable
Project Member

Comment 37 by bugdroid1@chromium.org, Mar 21 2013

Labels: -Security-Impact-Beta Security_Impact-Beta
Project Member

Comment 38 by bugdroid1@chromium.org, Apr 1 2013

Labels: -Performance-Memory-AddressSanitizer Stability-Memory-AddressSanitizer
Project Member

Comment 39 by sheriffbot@chromium.org, Jun 14 2016

Labels: -security_impact-beta
Project Member

Comment 40 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 41 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