1275339 - (CVE-2016-2839) Crash in _cairo_surface_get_extents with FFMPEG 0.10

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P1)

Description

[Bert Massop]

This bug was filed from the Socorro interface and is 
report bp-ca9ae074-6b5c-42f9-81eb-0080f2160524.
=============================================================

Firefox 46 consistently crashes with SIGSEGV when playing HTML5 video, or sometimes when merely loading HTML5 video elements.

Comment 1

[Loic]

Bert, do you have a testcase or steps to reproduce?

Comment 2

[Bert Massop]

I have reproduced it on a clean profile in bp-79b3bfcc-f9c3-4fa1-968c-ddb552160529.

1. Visit http://www.quirksmode.org/html5/tests/video.html
2. Click on the first video (under H.264/MP4) to start playback
3. Observe crash related.

N.B. The WebM and Ogg/Theora videos on the same page play just fine.

Comment 3

[Lee Salzman [:lsalzman]]

I tried to reproduce the bug on both Nightly and on 46 release, but I was not able to make either crash.

Could you paste the "Graphics" section of "about:support" here? That might give some more clues.

Comment 4

[Bert Massop]

Here's a copy-paste of said section. My locale is set to Dutch, rough translation of the items is given in [brackets].

Adapterbeschrijving [Adapter description]
nouveau -- Gallium 0.4 on NV96

Asynchroon pannen/zoomen [Async pan/zoom]
geen [none]

Device-ID
Gallium 0.4 on NV96

GPU-versnelde vensters [GPU-accelerated windows]
0/1 Basic (OMTC)

Ondersteunt hardwarematige H264-decodering [Supports hardware H264 decoding]
No;

Stuurprogrammaversie [Driver version]
3.0 Mesa 9.2.1

Vendor-ID
nouveau

WebGL-renderer
nouveau -- Gallium 0.4 on NV96

windowLayerManagerRemote
true

AzureCanvasBackend
cairo

AzureContentBackend
cairo

AzureFallbackCanvasBackend
none

AzureSkiaAccelerated
0

CairoUseXRender
1

Comment 5

[Lee Salzman [:lsalzman]]

Can you try running either developer edition or nightly and see if the problem still happens there?

Comment 6

[Bert Massop]

The same problem happens on a freshly downloaded Nightly build. Crash report is in bp-42d252a8-50d9-40be-9d4a-4c95a2160601.
Here, instead of crashing Firefox, only the single tab crashes.
In addition, the following is logged to stdout:

###!!! [Parent][MessageChannel] Error: (msgtype=0x2C007A,name=PBrowser::Msg_Destroy) Channel error: cannot send/recv

(But I suspect that to be an expected part of the tab crashing procedure, so to say.)

Interestingly, my second try (out of three tries) did not crash. Apparently this is not always reproducible.

Comment 7

[Bert Massop]

Tested Aurora just for the sake of it, crash report is in bp-d82ff05f-d844-4f93-8f6a-e0b522160601.

Comment 8

[Lee Salzman [:lsalzman]]

Still pondering this... My current guess is that whatever codec you are using may be causing some heap overflow in Firefox, or something in our media code not in Cairo is doing similar and/or misusing already freed memory.

I've tried to have a few people here reproduce the issue, but none of us have seemingly been able to.

What version of libavcodec do you have installed if you look in your list of installed packages? It is possible you have a much older version that is causing issues.

Comment 9

[Lee Salzman [:lsalzman]]

Chris, any possible guesses as to what might be involved here or someone on media who could look into this if something in our media code is causing it?

Comment 10

[Bert Massop]

Thanks for investigating this. I don't usually encounter trouble while playing videos in other players (mplayer, vlc, and earlier versions of Firefox - although I skipped a few versions of the latter, so I'm not sure this happens in the previous versions: will test)

According to `dpkg -s libavcodec53`, I have version 7:0.10.12-1~saucy1. Here's the output from `ffmpeg --version` as well:

ffmpeg version 0.10.12-7:0.10.12-1~saucy1
built on Apr 26 2014 09:54:16 with gcc 4.8.1
configuration: --arch=amd64 --disable-stripping --enable-pthreads --enable-runtime-cpudetect --extra-version='7:0.10.12-1~saucy1' --libdir=/usr/lib/x86_64-linux-gnu --prefix=/usr --enable-bzlib --enable-libdc1394 --enable-libfreetype --enable-frei0r --enable-gnutls --enable-libgsm --enable-libmp3lame --enable-librtmp --enable-libopencv --enable-libopenjpeg --enable-libpulse --enable-libschroedinger --enable-libspeex --enable-libtheora --enable-vaapi --enable-vdpau --enable-libvorbis --enable-libvpx --enable-zlib --enable-gpl --enable-postproc --enable-libcdio --enable-x11grab --enable-libx264 --shlibdir=/usr/lib/x86_64-linux-gnu --enable-shared --disable-static
libavutil      51. 35.100 / 51. 35.100
libavcodec     53. 61.100 / 53. 61.100
libavformat    53. 32.100 / 53. 32.100
libavdevice    53.  4.100 / 53.  4.100
libavfilter     2. 61.100 /  2. 61.100
libswscale      2.  1.100 /  2.  1.100
libswresample   0.  6.100 /  0.  6.100
libpostproc    52.  0.100 / 52.  0.100

Comment 11

[Bert Massop]

Results for 44.0.1:
bp-85581c48-9ba5-4453-abbb-288f22160601 (crashes immediately)
bp-838a1b11-c4a3-4e6c-aca0-a85982160601 (crashes a few seconds after start of play)
bp-6bfda755-db88-4537-9002-4b8302160601 (crashes a second after start of play)
Navigating immediately to http://www.quirksmode.org/html5/videos/big_buck_bunny.mp4 often (but not always) plays the file fine for a while on 44.0.1, but still crashes after refreshing the page (sometimes, several refreshes are required).
bp-fce1d8c9-511e-4642-a28b-f0e242160601 and bp-6d21f078-a9e9-42b0-b039-f11852160601 are related to that on 44.0.1.

Firefox 43.0 crashes intermittently.
First run, immediate crash: bp-042ea5a3-0c22-41bf-9589-df39a2160601
Second run plays fine, refresh the page, click the video, Firefox freezes but does not crash. Manually intervened by sending SIGABRT, resulting crashlog is bp-aa77c3b6-a1b3-4382-a070-62ee12160601.
Third, crashes immediately when loading the page: bp-9351fcd6-17d9-4ecd-ab87-cde9b2160601
Fourth run plays fine, but crashes when I refresh the page: bp-86ed4735-3d8e-4a22-b748-51b802160601

Firefox 42.0 appears to play the video just fine. I have not been able to cause a crash for it yet.

mplayer "http://www.quirksmode.org/html5/videos/big_buck_bunny.mp4" plays the video just fine
vlc "http://www.quirksmode.org/html5/videos/big_buck_bunny.mp4" plays the video just fine

Comment 12

[Lee Salzman [:lsalzman]]

(In reply to Bert Massop from comment #11)
> Results for 44.0.1:
> bp-85581c48-9ba5-4453-abbb-288f22160601 (crashes immediately)
> bp-838a1b11-c4a3-4e6c-aca0-a85982160601 (crashes a few seconds after start
> of play)
> bp-6bfda755-db88-4537-9002-4b8302160601 (crashes a second after start of
> play)
> Navigating immediately to
> http://www.quirksmode.org/html5/videos/big_buck_bunny.mp4 often (but not
> always) plays the file fine for a while on 44.0.1, but still crashes after
> refreshing the page (sometimes, several refreshes are required).
> bp-fce1d8c9-511e-4642-a28b-f0e242160601 and
> bp-6d21f078-a9e9-42b0-b039-f11852160601 are related to that on 44.0.1.
> 
> Firefox 43.0 crashes intermittently.
> First run, immediate crash: bp-042ea5a3-0c22-41bf-9589-df39a2160601
> Second run plays fine, refresh the page, click the video, Firefox freezes
> but does not crash. Manually intervened by sending SIGABRT, resulting
> crashlog is bp-aa77c3b6-a1b3-4382-a070-62ee12160601.
> Third, crashes immediately when loading the page:
> bp-9351fcd6-17d9-4ecd-ab87-cde9b2160601
> Fourth run plays fine, but crashes when I refresh the page:
> bp-86ed4735-3d8e-4a22-b748-51b802160601
> 
> Firefox 42.0 appears to play the video just fine. I have not been able to
> cause a crash for it yet.
> 
> mplayer "http://www.quirksmode.org/html5/videos/big_buck_bunny.mp4" plays
> the video just fine
> vlc "http://www.quirksmode.org/html5/videos/big_buck_bunny.mp4" plays the
> video just fine

So presuming this is some change we made, could you try mozregression and see if you can narrow down what change caused the issue?

http://mozilla.github.io/mozregression/

Since so far only you can reproduce the issue reliably, this is going to be the easiest way to narrow it down for us.

Comment 13

[Bert Massop]

This bug seems to be slightly harder to reproduce on the affected nightlies / inbounds. Some abuse of play/pause and refresh still allowed me to bisect.

Bisection narrowed the introduction of the regression down to revision 9a4457502650. That doesn't surprise me, given its content, but it does not yield a lot of relevant information either. Except that it's probably memory-related, but that was already likely to be the case.

Comment 14

[Bert Massop]

Even though possibly unrelated, these log lines captured my attention, they are displayed whenever the page is (re)loaded (but before playing the video):
(running mozilla-central build for 2015-12-14)

2016-06-02T00:02:06: INFO : thread '<unnamed>' panicked at 'assertion failed: content.limit() == 0', /builds/slave/m-cen-l64-ntly-000000000000000/build/src/media/libstagefright/binding/MP4Metadata.rs:394
2016-06-02T00:02:06: INFO : thread '<unnamed>' panicked at 'assertion failed: content.limit() == 0', /builds/slave/m-cen-l64-ntly-000000000000000/build/src/media/libstagefright/binding/MP4Metadata.rs:394
2016-06-02T00:02:06: INFO : thread '<unnamed>' panicked at 'assertion failed: content.limit() == 0', /builds/slave/m-cen-l64-ntly-000000000000000/build/src/media/libstagefright/binding/MP4Metadata.rs:394
2016-06-02T00:02:06: INFO : thread '<unnamed>' panicked at 'assertion failed: content.limit() == 0', /builds/slave/m-cen-l64-ntly-000000000000000/build/src/media/libstagefright/binding/MP4Metadata.rs:394
2016-06-02T00:02:06: INFO : thread '<unnamed>' panicked at 'assertion failed: content.limit() == 0', /builds/slave/m-cen-l64-ntly-000000000000000/build/src/media/libstagefright/binding/MP4Metadata.rs:394

Comment 15

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Lee Salzman [:lsalzman] from comment #9)
> Chris, any possible guesses as to what might be involved here or someone on
> media who could look into this if something in our media code is causing it?

This is a crash in cairo, the rendering layer. It's not a media playback issue. It could be that the video playback code is outputting a frame of geometry that cairo can't handle maybe.

Someone on the graphics team should look into this to diagnose.

Comment 16

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Bert Massop from comment #14)
> Even though possibly unrelated, these log lines captured my attention, they
> are displayed whenever the page is (re)loaded (but before playing the video):
> (running mozilla-central build for 2015-12-14)
> 
> 2016-06-02T00:02:06: INFO : thread '<unnamed>' panicked at 'assertion
> failed: content.limit() == 0',
> /builds/slave/m-cen-l64-ntly-000000000000000/build/src/media/libstagefright/
> binding/MP4Metadata.rs:394
> 2016-06-02T00:02:06: INFO : thread '<unnamed>' panicked at 'assertion
> failed: content.limit() == 0',
> /builds/slave/m-cen-l64-ntly-000000000000000/build/src/media/libstagefright/
> binding/MP4Metadata.rs:394
> 2016-06-02T00:02:06: INFO : thread '<unnamed>' panicked at 'assertion
> failed: content.limit() == 0',
> /builds/slave/m-cen-l64-ntly-000000000000000/build/src/media/libstagefright/
> binding/MP4Metadata.rs:394
> 2016-06-02T00:02:06: INFO : thread '<unnamed>' panicked at 'assertion
> failed: content.limit() == 0',
> /builds/slave/m-cen-l64-ntly-000000000000000/build/src/media/libstagefright/
> binding/MP4Metadata.rs:394
> 2016-06-02T00:02:06: INFO : thread '<unnamed>' panicked at 'assertion
> failed: content.limit() == 0',
> /builds/slave/m-cen-l64-ntly-000000000000000/build/src/media/libstagefright/
> binding/MP4Metadata.rs:394

These assertions are harmless; we run the Rust MP4 demuxer and compare the output with the output of our C++ MP4 demuxer, and report via telemetry whether the Rust demuxer produced the same result. We use the C++ MP4 demuxer's output. So these assertions have no impact on behaviour.

Comment 17

[Lee Salzman [:lsalzman]]

Attachment: check if creation of cairo surface fails in DrawTargetCairo::DrawSurface

I don't think this will fix the problem, but it is none the less an incidental finding. Creating a pattern with a null surface will result in an error pattern getting drawn which may cause problems downwind.

Comment 18

[Lee Salzman [:lsalzman]]

Might you be willing to try upgrading your ffmpeg/libavcodec to a newer version? It might eliminate the hunch that the codec itself is somehow involved in this. It's really my best guess.

I've done as much investigation as I can at this point, tried getting other people to see if they can reproduce the crash without luck, etc. I can't really find any smoking guns in the graphics code, especially since some of your stack traces don't end up in Cairo (suspiciously one being in the ffmmpeg code).

Comment 19

[Milan Sreckovic [:milan] (needinfo for best results)]

Comment on attachment 8759185 [details] [diff] [review]
check if creation of cairo surface fails in DrawTargetCairo::DrawSurface

Review of attachment 8759185 [details] [diff] [review]:
-----------------------------------------------------------------

Warning is probably enough given that we have somebody that can reproduce this; to see the warnings though, they'd have to set gfx.logging.level to 3 or 4.

Comment 20

[Milan Sreckovic [:milan] (needinfo for best results)]

Anthony, can somebody with the knowledge of video help here?  We don't know where to take this bug, and seems to be highly video related so some expertise on that front would help.  At ~100 crashes a day, we shouldn't ignore this, and we have somebody that can reproduce the problem.

Comment 21

[Bert Massop]

Comment on attachment 8759185 [details] [diff] [review]
check if creation of cairo surface fails in DrawTargetCairo::DrawSurface

Does not compile when applied against 46.0.1. Shouldn't this read "gfxWarning() << ..."?

Comment 22

[Bert Massop]

This problem is solved by using the libav[codec, format, ...] from the latest ffmpeg 3.0.2 (avcodec 57; compiled with minimal configure flags: disable-static, enable-shared, enable-pic), and I can reproduce it with those from ffmpeg 0.10.12 (avcodec 53; compiled with the same settings).

Results for different versions of ffmpeg (and thereby libavcodec) are as follows:
0.10.12 (avcodec 53.61.100): crash
0.11 (avcodec 54.23.100): file type not supported
0.11.5 (avcodec 54.23.100): file type not supported
1.0 (avcodec 54.59.100): good

Comment 23

[Bert Massop]

Forgot the latest line:

3.0.2 (avcodec 57.24.102): good

Comment 24

[Lee Salzman [:lsalzman]]

Attachment: check if creation of cairo surface fails in DrawTargetCairo::DrawSurface

Fixed missing parens.

Comment 25

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Jean-Yves - can you take a look at this?

Comment 26

[Jean-Yves Avenard [:jya]]

FFmpeg 0.10 was crashing heaps in my experience, due to our custom memory allocation. It should have been fixed by bug 1236746 which was uplifted to 45.

will investigate

Comment 27

[Jean-Yves Avenard [:jya]]

same error with LibAV9

==19876==AddressSanitizer: while reporting a bug found another one. Ignoring.
[1m[31m==19876==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6130007d1e30 at pc 0x00010012164a bp 0x700005005d60 sp 0x700005005520
[1m[0m[1m[34mWRITE of size 384 at 0x6130007d1e30 thread T114[1m[0m
    #0 0x100121649 in wrap_memset (libclang_rt.asan_osx_dynamic.dylib+0x3c649)
    #1 0x7fff96c49a6a in __memset_chk (libsystem_c.dylib+0x84a6a)
    #2 0x1bb7ba384 in avcodec_get_frame_defaults (libavcodec.53.dylib+0x39e384)
    #3 0x110adac2d in mozilla::FFmpegDataDecoder<53>::PrepareFrame() FFmpegDataDecoder.cpp:213
    #4 0x110ad9c70 in mozilla::FFmpegAudioDecoder<53>::DoDecode(mozilla::MediaRawData*) FFmpegAudioDecoder.cpp:108
    #5 0x110adbddc in mozilla::FFmpegDataDecoder<53>::ProcessDecode(mozilla::MediaRawData*) FFmpegDataDecoder.cpp:122
    #6 0x110ae741c in decltype(*(fp).*fp0(mozilla::Get<0ul>(fp1).PassAsParameter())) nsRunnableMethodArguments<RefPtr<mozilla::MediaRawData> >::applyImpl<mozilla::FFmpegDataDecoder<53>, void (mozilla::FFmpegDataDecoder<53>::*)(mozilla::MediaRawData*), StorensRefPtrPassByPtr<mozilla::MediaRawData>, 0ul>(mozilla::FFmpegDataDecoder<53>*, void (mozilla::FFmpegDataDecoder<53>::*)(mozilla::MediaRawData*), mozilla::Tuple<StorensRefPtrPassByPtr<mozilla::MediaRawData> >&, mozilla::IndexSequence<0ul>) nsThreadUtils.h:715
    #7 0x110ae710e in _ZN25nsRunnableMethodArgumentsIJ6RefPtrIN7mozilla12MediaRawDataEEEE5applyINS1_17FFmpegDataDecoderILi53EEEMS7_FvPS2_EEEDTcl9applyImplfp_fp0_dtdefpT10mArgumentscvNS1_13IndexSequenceIJLm0EEEE_EEEPT_T0_ nsThreadUtils.h:721
    #8 0x110ae6d90 in nsRunnableMethodImpl<void (mozilla::FFmpegDataDecoder<53>::*)(mozilla::MediaRawData*), true, false, RefPtr<mozilla::MediaRawData> >::Run() nsThreadUtils.h:749
    #9 0x106c70ca3 in mozilla::TaskQueue::Runner::Run() TaskQueue.cpp:172
    #10 0x106c8d763 in nsThreadPool::Run() nsThreadPool.cpp:227
    #11 0x106c8560c in nsThread::ProcessNextEvent(bool, bool*) nsThread.cpp:1029
    #12 0x106de2a45 in NS_ProcessNextEvent(nsIThread*, bool) nsThreadUtils.cpp:290
    #13 0x10874afad in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) MessagePump.cpp:354
    #14 0x10847e1ca in MessageLoop::RunInternal() message_loop.cc:235
    #15 0x10847e004 in MessageLoop::RunHandler() message_loop.cc:228
    #16 0x10847df41 in MessageLoop::Run() message_loop.cc:208
    #17 0x106c7ed46 in nsThread::ThreadFunc(void*) nsThread.cpp:466
    #18 0x1062905b5 in _pt_root ptthread.c:216
    #19 0x100eab804 in _pthread_body (libsystem_pthread.dylib+0x3804)
    #20 0x100eab781 in _pthread_start (libsystem_pthread.dylib+0x3781)
    #21 0x100ea8fa0 in thread_start (libsystem_pthread.dylib+0xfa0)

[1m[32m0x6130007d1e30 is located 0 bytes to the right of 368-byte region [0x6130007d1cc0,0x6130007d1e30)
[1m[0m[1m[35mallocated by thread T114 here:[1m[0m
    #0 0x10012d9c0 in wrap_malloc (libclang_rt.asan_osx_dynamic.dylib+0x489c0)
    #1 0x10004b534 in moz_xmalloc mozalloc.cpp:83
    #2 0x110adab3b in mozilla::FFmpegDataDecoder<53>::PrepareFrame() mozalloc.h:193
    #3 0x110ad9c70 in mozilla::FFmpegAudioDecoder<53>::DoDecode(mozilla::MediaRawData*) FFmpegAudioDecoder.cpp:108
    #4 0x110adbddc in mozilla::FFmpegDataDecoder<53>::ProcessDecode(mozilla::MediaRawData*) FFmpegDataDecoder.cpp:122
    #5 0x110ae741c in decltype(*(fp).*fp0(mozilla::Get<0ul>(fp1).PassAsParameter())) nsRunnableMethodArguments<RefPtr<mozilla::MediaRawData> >::applyImpl<mozilla::FFmpegDataDecoder<53>, void (mozilla::FFmpegDataDecoder<53>::*)(mozilla::MediaRawData*), StorensRefPtrPassByPtr<mozilla::MediaRawData>, 0ul>(mozilla::FFmpegDataDecoder<53>*, void (mozilla::FFmpegDataDecoder<53>::*)(mozilla::MediaRawData*), mozilla::Tuple<StorensRefPtrPassByPtr<mozilla::MediaRawData> >&, mozilla::IndexSequence<0ul>) nsThreadUtils.h:715
    #6 0x110ae710e in _ZN25nsRunnableMethodArgumentsIJ6RefPtrIN7mozilla12MediaRawDataEEEE5applyINS1_17FFmpegDataDecoderILi53EEEMS7_FvPS2_EEEDTcl9applyImplfp_fp0_dtdefpT10mArgumentscvNS1_13IndexSequenceIJLm0EEEE_EEEPT_T0_ nsThreadUtils.h:721
    #7 0x110ae6d90 in nsRunnableMethodImpl<void (mozilla::FFmpegDataDecoder<53>::*)(mozilla::MediaRawData*), true, false, RefPtr<mozilla::MediaRawData> >::Run() nsThreadUtils.h:749
    #8 0x106c70ca3 in mozilla::TaskQueue::Runner::Run() TaskQueue.cpp:172
    #9 0x106c8d763 in nsThreadPool::Run() nsThreadPool.cpp:227
    #10 0x106c8560c in nsThread::ProcessNextEvent(bool, bool*) nsThread.cpp:1029
    #11 0x106de2a45 in NS_ProcessNextEvent(nsIThread*, bool) nsThreadUtils.cpp:290
    #12 0x10874afad in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) MessagePump.cpp:354
    #13 0x10847e1ca in MessageLoop::RunInternal() message_loop.cc:235
    #14 0x10847e004 in MessageLoop::RunHandler() message_loop.cc:228
    #15 0x10847df41 in MessageLoop::Run() message_loop.cc:208
    #16 0x106c7ed46 in nsThread::ThreadFunc(void*) nsThread.cpp:466
    #17 0x1062905b5 in _pt_root ptthread.c:216
    #18 0x100eab804 in _pthread_body (libsystem_pthread.dylib+0x3804)
    #19 0x100eab781 in _pthread_start (libsystem_pthread.dylib+0x3781)
    #20 0x100ea8fa0 in thread_start (libsystem_pthread.dylib+0xfa0)

Thread T114 created by T111 here:
    #0 0x100123f99 in wrap_pthread_create (libclang_rt.asan_osx_dynamic.dylib+0x3ef99)
    #1 0x10628aa5b in _PR_CreateThread ptthread.c:457
    #2 0x106289e73 in PR_CreateThread ptthread.c:548
    #3 0x106c80c24 in nsThread::Init() nsThread.cpp:600
    #4 0x106c89ddd in nsThreadManager::NewThread(unsigned int, unsigned int, nsIThread**) nsThreadManager.cpp:253
    #5 0x106c8c44c in nsThreadPool::PutEvent(already_AddRefed<nsIRunnable>, unsigned int) nsThreadPool.cpp:106
    #6 0x106c8e066 in nsThreadPool::Dispatch(already_AddRefed<nsIRunnable>, unsigned int) nsThreadPool.cpp:276
    #7 0x106ca050a in mozilla::SharedThreadPool::Dispatch(already_AddRefed<nsIRunnable>, unsigned int) SharedThreadPool.h:68
    #8 0x106c6f35a in mozilla::TaskQueue::DispatchLocked(nsCOMPtr<nsIRunnable>&, mozilla::TaskQueue::DispatchMode, mozilla::AbstractThread::DispatchFailureHandling, mozilla::AbstractThread::DispatchReason) TaskQueue.cpp:67
    #9 0x106ca112e in mozilla::TaskQueue::Dispatch(already_AddRefed<nsIRunnable>, mozilla::AbstractThread::DispatchFailureHandling, mozilla::AbstractThread::DispatchReason) TaskQueue.h:49
    #10 0x110adc389 in mozilla::FFmpegDataDecoder<53>::Input(mozilla::MediaRawData*) FFmpegDataDecoder.cpp:139
    #11 0x1105d4049 in mozilla::MediaFormatReader::DecodeDemuxedSamples(mozilla::TrackInfo::TrackType, mozilla::MediaRawData*) MediaFormatReader.cpp:908
    #12 0x1105d545c in mozilla::MediaFormatReader::HandleDemuxedSamples(mozilla::TrackInfo::TrackType, mozilla::AbstractMediaDecoder::AutoNotifyDecoded&) MediaFormatReader.cpp:1003
    #13 0x1105d1e8d in mozilla::MediaFormatReader::Update(mozilla::TrackInfo::TrackType) MediaFormatReader.cpp:1264
    #14 0x1106e625b in decltype(*(fp).*fp0(mozilla::Get<0ul>(fp1).PassAsParameter())) nsRunnableMethodArguments<mozilla::TrackInfo::TrackType>::applyImpl<mozilla::MediaFormatReader, void (mozilla::MediaFormatReader::*)(mozilla::TrackInfo::TrackType), StoreCopyPassByValue<mozilla::TrackInfo::TrackType>, 0ul>(mozilla::MediaFormatReader*, void (mozilla::MediaFormatReader::*)(mozilla::TrackInfo::TrackType), mozilla::Tuple<StoreCopyPassByValue<mozilla::TrackInfo::TrackType> >&, mozilla::IndexSequence<0ul>) nsThreadUtils.h:715
    #15 0x1106e5f4e in _ZN25nsRunnableMethodArgumentsIJN7mozilla9TrackInfo9TrackTypeEEE5applyINS0_17MediaFormatReaderEMS5_FvS2_EEEDTcl9applyImplfp_fp0_dtdefpT10mArgumentscvNS0_13IndexSequenceIJLm0EEEE_EEEPT_T0_ nsThreadUtils.h:721
    #16 0x1106e5920 in nsRunnableMethodImpl<void (mozilla::MediaFormatReader::*)(mozilla::TrackInfo::TrackType), true, false, mozilla::TrackInfo::TrackType>::Run() nsThreadUtils.h:749
    #17 0x106ca4f32 in mozilla::AutoTaskDispatcher::TaskGroupRunnable::Run() TaskDispatcher.h:192
    #18 0x106c70ca3 in mozilla::TaskQueue::Runner::Run() TaskQueue.cpp:172
    #19 0x106c8d763 in nsThreadPool::Run() nsThreadPool.cpp:227
    #20 0x106c8560c in nsThread::ProcessNextEvent(bool, bool*) nsThread.cpp:1029
    #21 0x106de2a45 in NS_ProcessNextEvent(nsIThread*, bool) nsThreadUtils.cpp:290
    #22 0x10874afad in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) MessagePump.cpp:354
    #23 0x10847e1ca in MessageLoop::RunInternal() message_loop.cc:235
    #24 0x10847e004 in MessageLoop::RunHandler() message_loop.cc:228
    #25 0x10847df41 in MessageLoop::Run() message_loop.cc:208
    #26 0x106c7ed46 in nsThread::ThreadFunc(void*) nsThread.cpp:466
    #27 0x1062905b5 in _pt_root ptthread.c:216
    #28 0x100eab804 in _pthread_body (libsystem_pthread.dylib+0x3804)
    #29 0x100eab781 in _pthread_start (libsystem_pthread.dylib+0x3781)
    #30 0x100ea8fa0 in thread_start (libsystem_pthread.dylib+0xfa0)

Thread T111 created by T0 here:
    #0 0x100123f99 in wrap_pthread_create (libclang_rt.asan_osx_dynamic.dylib+0x3ef99)
    #1 0x10628aa5b in _PR_CreateThread ptthread.c:457
    #2 0x106289e73 in PR_CreateThread ptthread.c:548
    #3 0x106c80c24 in nsThread::Init() nsThread.cpp:600
    #4 0x106c89ddd in nsThreadManager::NewThread(unsigned int, unsigned int, nsIThread**) nsThreadManager.cpp:253
    #5 0x106c8c44c in nsThreadPool::PutEvent(already_AddRefed<nsIRunnable>, unsigned int) nsThreadPool.cpp:106
    #6 0x106c8e066 in nsThreadPool::Dispatch(already_AddRefed<nsIRunnable>, unsigned int) nsThreadPool.cpp:276
    #7 0x106ca050a in mozilla::SharedThreadPool::Dispatch(already_AddRefed<nsIRunnable>, unsigned int) SharedThreadPool.h:68
    #8 0x106c6f35a in mozilla::TaskQueue::DispatchLocked(nsCOMPtr<nsIRunnable>&, mozilla::TaskQueue::DispatchMode, mozilla::AbstractThread::DispatchFailureHandling, mozilla::AbstractThread::DispatchReason) TaskQueue.cpp:67
    #9 0x106ca112e in mozilla::TaskQueue::Dispatch(already_AddRefed<nsIRunnable>, mozilla::AbstractThread::DispatchFailureHandling, mozilla::AbstractThread::DispatchReason) TaskQueue.h:49
    #10 0x106ca4334 in mozilla::AutoTaskDispatcher::DispatchTaskGroup(mozilla::UniquePtr<mozilla::AutoTaskDispatcher::PerThreadTaskGroup, mozilla::DefaultDelete<mozilla::AutoTaskDispatcher::PerThreadTaskGroup> >) TaskDispatcher.h:244
    #11 0x106ca3ec2 in mozilla::AutoTaskDispatcher::~AutoTaskDispatcher() TaskDispatcher.h:90
    #12 0x106ca3274 in mozilla::AutoTaskDispatcher::~AutoTaskDispatcher() TaskDispatcher.h:77
    #13 0x106ca93ca in mozilla::Maybe<mozilla::AutoTaskDispatcher>::reset() Maybe.h:373
    #14 0x106ca2e9f in mozilla::XPCOMThreadWrapper::FireTailDispatcher() AbstractThread.cpp:81
    #15 0x106ca8dfd in decltype(*(fp).*fp0(mozilla::Get<>(fp1).PassAsParameter())) nsRunnableMethodArguments<>::applyImpl<mozilla::XPCOMThreadWrapper, void (mozilla::XPCOMThreadWrapper::*)()>(mozilla::XPCOMThreadWrapper*, void (mozilla::XPCOMThreadWrapper::*)(), mozilla::Tuple<>&, mozilla::IndexSequence<>) nsThreadUtils.h:715
    #16 0x106ca8abe in _ZN25nsRunnableMethodArgumentsIJEE5applyIN7mozilla18XPCOMThreadWrapperEMS3_FvvEEEDTcl9applyImplfp_fp0_dtdefpT10mArgumentscvNS2_13IndexSequenceIJEEE_EEEPT_T0_ nsThreadUtils.h:721
    #17 0x106ca8680 in nsRunnableMethodImpl<void (mozilla::XPCOMThreadWrapper::*)(), true, false>::Run() nsThreadUtils.h:749
    #18 0x106a119d9 in mozilla::CycleCollectedJSRuntime::ProcessStableStateQueue() CycleCollectedJSRuntime.cpp:1301
    #19 0x106a19ce3 in mozilla::CycleCollectedJSRuntime::AfterProcessTask(unsigned int) CycleCollectedJSRuntime.cpp:1359
    #20 0x109b0f5fb in XPCJSRuntime::AfterProcessTask(unsigned int) XPCJSRuntime.cpp:3753
    #21 0x106c85b30 in nsThread::ProcessNextEvent(bool, bool*) nsThread.cpp:1044
    #22 0x106de26f6 in NS_ProcessPendingEvents(nsIThread*, unsigned int) nsThreadUtils.cpp:232
    #23 0x1121e26a8 in nsBaseAppShell::NativeEventCallback() nsBaseAppShell.cpp:97
    #24 0x11235b09c in nsAppShell::ProcessGeckoEvents(void*) nsAppShell.mm:387
    #25 0x7fff95c38880 in __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ (CoreFoundation+0xaa880)
    #26 0x7fff95c17f36 in __CFRunLoopDoSources0 (CoreFoundation+0x89f36)
    #27 0x7fff95c174de in __CFRunLoopRun (CoreFoundation+0x894de)
    #28 0x7fff95c16ed7 in CFRunLoopRunSpecific (CoreFoundation+0x88ed7)
    #29 0x7fff9c245934 in RunCurrentEventLoopInMode (HIToolbox+0x30934)
    #30 0x7fff9c245676 in ReceiveNextEventCommon (HIToolbox+0x30676)
    #31 0x7fff9c2455ae in _BlockUntilNextEventMatchingListInModeWithFilter (HIToolbox+0x305ae)
    #32 0x7fff9149ddf5 in _DPSNextEvent (AppKit+0x48df5)
    #33 0x7fff9149d225 in -[NSApplication _nextEventMatchingEventMask:untilDate:inMode:dequeue:] (AppKit+0x48225)
    #34 0x11235816a in -[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] nsAppShell.mm:121
    #35 0x7fff91491d7f in -[NSApplication run] (AppKit+0x3cd7f)
    #36 0x11235ca24 in nsAppShell::Run() nsAppShell.mm:661
    #37 0x11572494f in nsAppStartup::Run() nsAppStartup.cpp:284
    #38 0x1158d56c7 in XREMain::XRE_mainRun() nsAppRunner.cpp:4374
    #39 0x1158d7960 in XREMain::XRE_main(int, char**, nsXREAppData const*) nsAppRunner.cpp:4478
    #40 0x1158d82ce in XRE_main nsAppRunner.cpp:4586
    #41 0x100003ec5 in do_main(int, char**, char**, nsIFile*) nsBrowserApp.cpp:242
    #42 0x100002595 in main nsBrowserApp.cpp:382
    #43 0x100000ec3 in start (firefox+0x100000ec3)
    #44 0x6  (firefox)+0x6)

SUMMARY: AddressSanitizer: heap-buffer-overflow (libclang_rt.asan_osx_dynamic.dylib+0x3c649) in wrap_memset
Shadow bytes around the buggy address:
  0x1c26000fa370: [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m
  0x1c26000fa380: [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m
  0x1c26000fa390: [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m
  0x1c26000fa3a0: [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m
  0x1c26000fa3b0: [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m
=>0x1c26000fa3c0: [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m[[1m[31mfa[1m[0m][1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m
  0x1c26000fa3d0: [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m
  0x1c26000fa3e0: [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m
  0x1c26000fa3f0: [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[0m00[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m
  0x1c26000fa400: [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m
  0x1c26000fa410: [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m [1m[31mfa[1m[0m
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           [1m[0m00[1m[0m
  Partially addressable: [1m[0m01[1m[0m [1m[0m02[1m[0m [1m[0m03[1m[0m [1m[0m04[1m[0m [1m[0m05[1m[0m [1m[0m06[1m[0m [1m[0m07[1m[0m 
  Heap left redzone:       [1m[31mfa[1m[0m
  Heap right redzone:      [1m[31mfb[1m[0m
  Freed heap region:       [1m[35mfd[1m[0m
  Stack left redzone:      [1m[31mf1[1m[0m
  Stack mid redzone:       [1m[31mf2[1m[0m
  Stack right redzone:     [1m[31mf3[1m[0m
  Stack partial redzone:   [1m[31mf4[1m[0m
  Stack after return:      [1m[35mf5[1m[0m
  Stack use after scope:   [1m[35mf8[1m[0m
  Global redzone:          [1m[31mf9[1m[0m
  Global init order:       [1m[36mf6[1m[0m
  Poisoned by user:        [1m[34mf7[1m[0m
  Container overflow:      [1m[34mfc[1m[0m
  Array cookie:            [1m[31mac[1m[0m
  Intra object redzone:    [1m[33mbb[1m[0m
  ASan internal:           [1m[33mfe[1m[0m
  Left alloca redzone:     [1m[34mca[1m[0m
  Right alloca redzone:    [1m[34mcb[1m[0m
==19876==ABORTING
Warning: hit breakpoint while running function, skipping commands and conditions to prevent recursion.AddressSanitizer report breakpoint hit. Use 'thread info -s' to get extended information about the report.
(lldb)

Comment 28

[Jean-Yves Avenard [:jya]]

Attachment: [ffmpeg] Don't assume AVFrame has a constant size.

An AVFrame has a different size between FFmpeg 0.10 and LibAV 0.8 though both have the same version number.

I guess the comment " * sizeof(AVFrame) must not be used outside libav*." was rather explicit. Yet, got overlooked (this code is from the original FFmpeg (LibAV really) PDM implementation.
An AVFrame is 16 bytes bigger on FFmpeg 0.10 than LibAV 0.8/9

Comment 29

[Jean-Yves Avenard [:jya]]

Attachment: [ffmpeg] Don't assume AVFrame has a constant size.

An AVFrame has a different size between FFmpeg 0.10 and LibAV 0.8 though both have the same version number.

MozReview-Commit-ID: JflzY0m9urb

Comment 30

[Jean-Yves Avenard [:jya]]

Comment on attachment 8760620 [details] [diff] [review]
[ffmpeg] Don't assume AVFrame has a constant size.

[Security approval request comment]
How easily could an exploit be constructed based on the patch? no idea if it can be. Plus I'm not aware of any linux distribution using FFmpeg 0.10 (debian/ubuntu use LibAV 0.8 or LibAV 9. They only switched to FFmpeg . AVFrame is 16 bytes longer on the FFmpeg branch. We used the LibAV header so the AVFrame size allocated was 16 bytes short.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? yes. very generic comment

Which older supported branches are affected by this flaw? bug was introduced in bug 1041950 (my 2nd patch ever at Mozilla yippee yeah!), though FFmpeg didn't get enabled until late last year.

If not all supported branches, which bug introduced the flaw? bug 1041950

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? As of today, the fix should be the same in all released branches

How likely is this patch to cause regressions; how much testing does it need? don't think it needs any. manual tested, and going by the documentation for once

Comment 31

[Bert Massop]

Attachment 8760620 [details] [diff] doesn't compile when applied against the 46.0.1 release source. Should I test against another source version?

 3:18.49 In file included from /home/bert/firefox-46.0.1/obj-x86_64-unknown-linux-gnu/dom/media/platforms/ffmpeg/libav53/Unified_cpp_ffmpeg_libav530.cpp:11:0:
 3:18.87 /home/bert/firefox-46.0.1/dom/media/platforms/ffmpeg/FFmpegDataDecoder.cpp: In member function ‘AVFrame* mozilla::FFmpegDataDecoder<53>::PrepareFrame()’:
 3:18.87 /home/bert/firefox-46.0.1/dom/media/platforms/ffmpeg/FFmpegDataDecoder.cpp:188:9: error: ‘struct mozilla::FFmpegLibWrapper’ has no member named ‘av_free’
 3:18.87    mLib->av_free(mFrame);
 3:18.87          ^
 3:21.01 
 3:21.01 In the directory  /home/bert/firefox-46.0.1/obj-x86_64-unknown-linux-gnu/dom/media/platforms/ffmpeg/libav53
 3:21.01 The following command failed to execute properly:
 3:21.01 c++ -o Unified_cpp_ffmpeg_libav530.o [...]

Comment 32

[Jean-Yves Avenard [:jya]]

let me wrap something for <= 46.

Comment 33

[Jean-Yves Avenard [:jya]]

actually, you used the first patch, I had done a quick optimisation before pushing (av_free vs av_freep, but the symbol av_free isn't resolved by the dynamic linker).

the new patch will apply in 47.

Comment 34

[Jean-Yves Avenard [:jya]]

Which linux distribution are you using btw? I'm guessing it's a version of ffmpeg you either compiled yourself or used a 3rd party repository

Comment 35

[Jean-Yves Avenard [:jya]]

Attachment: [ffmpeg] Don't assume AVFrame has a constant size.

An AVFrame has a different size between FFmpeg 0.10 and LibAV 0.8 though both have the same version number.

Forgot to add the fixes (twice now :( )

Comment 36

[Jean-Yves Avenard [:jya]]

Comment on attachment 8760641 [details] [diff] [review]
[ffmpeg] Don't assume AVFrame has a constant size.

[Security approval request comment]
How easily could an exploit be constructed based on the patch? no idea if it can be. Plus I'm not aware of any linux distribution using FFmpeg 0.10 (debian/ubuntu use LibAV 0.8 or LibAV 9. They only switched to FFmpeg . AVFrame is 16 bytes longer on the FFmpeg branch. We used the LibAV header so the AVFrame size allocated was 16 bytes short.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? yes. very generic comment

Which older supported branches are affected by this flaw? bug was introduced in bug 1041950 (my 2nd patch ever at Mozilla yippee yeah!), though FFmpeg didn't get enabled until late last year.

If not all supported branches, which bug introduced the flaw? bug 1041950

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? As of today, the fix should be the same in all released branches but esr45 (it will work if >= 47)

How likely is this patch to cause regressions; how much testing does it need? don't think it needs any. manual tested, and going by the documentation for once

Comment 37

[Bert Massop]

(In reply to Jean-Yves Avenard [:jya] from comment #34)
> Which linux distribution are you using btw? I'm guessing it's a version of
> ffmpeg you either compiled yourself or used a 3rd party repository

I'm using Ubuntu "Saucy" 13.10. Yes, it's outdated and I should upgrade.

ffmpeg 0.10.12 is probably from Jon Severinsson's PPA because I once needed ffmpeg > 0.8.6 for $reasons. At the time, this was considered the primary PPA source for ffmpeg updates for Ubuntu.

Comment 38

[Jean-Yves Avenard [:jya]]

Attachment: [ffmpeg] Don't assume AVFrame has a constant size. esr45 patch

An AVFrame has a different size between FFmpeg 0.10 and LibAV 0.8 though both have the same version number.

MozReview-Commit-ID: JflzY0m9urb

Comment 39

[Jean-Yves Avenard [:jya]]

Attachment: [ffmpeg] P2. Fetch avcodec_free_frame in libavcodec.

avcodec_free_frame was incorrectly looked in libavuti. On Linux where this code is used, it makes no difference really but for clarity sake.

MozReview-Commit-ID: DC1WJAAKiE4

Comment 40

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Chris Pearce (:cpearce) from comment #15)
> 
> This is a crash in cairo, the rendering layer. It's not a media playback
> issue. It could be that the video playback code is outputting a frame of
> geometry that cairo can't handle maybe.

Nice guess as to what the eventual problem was, good intuition!

Lee, thanks for chasing this in the first place, media team, thanks for the fix, handling the crash and security bug in one.

Comment 41

[Al Billings [:abillings - ex-MoCo]]

This needs a security rating to get sec-approval and I'm not sure what to rate this with all the caveats in the approval request. Dan?

Comment 42

[Daniel Veditz [:dveditz] (OOO: back Aug 26)]

I'll call it "sec-moderate" overall.

We can clear the "leave-open" keyword now, right?

Comment 43

[Al Billings [:abillings - ex-MoCo]]

Comment on attachment 8760641 [details] [diff] [review]
[ffmpeg] Don't assume AVFrame has a constant size.

Clearing sec-approval. As a sec-moderate, this can just go in.

Comment 44

[Jean-Yves Avenard [:jya]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/1bafbbad7fa4
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/9ddfddc33c56

Comment 45

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3657c61116fa
https://hg.mozilla.org/mozilla-central/rev/1bafbbad7fa4
https://hg.mozilla.org/mozilla-central/rev/9ddfddc33c56

Comment 46

[Jean-Yves Avenard [:jya]]

Comment on attachment 8760641 [details] [diff] [review]
[ffmpeg] Don't assume AVFrame has a constant size.

Approval Request Comment
[Feature/regressing bug #]: 1041950
[User impact if declined]: crashes with some version of ffmpeg installed. 
[Describe test coverage new/current, TreeHerder]: in central, manual tests, verified by reporter.
[Risks and why]: low. Going by the API documentation. 
[String/UUID change made/needed]: none

Comment 47

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1c3037d99f3af964fb481169d9eb6b65a81bd081
https://hg.mozilla.org/releases/mozilla-beta/rev/58590eb627383ea4df6dcbef985adefcac0dae91

Comment 48

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Jean-Yves, I noticed that ers45 is marked as affected. Should we consider uplifting this to ESR45?

Comment 49

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Al, Dan, this is a sec-mod bug. Should we consider uplifting the fix to esr45 that is marked as affected? Or is this a wontfix? I wanted to get your thoughts on this. Thanks!

Comment 50

[Jean-Yves Avenard [:jya]]

Yes it would be good

Comment 51

[Al Billings [:abillings - ex-MoCo]]

This seems like a pretty constrained patch so I see no reason we could not take it on ESR45.

Comment 52

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

(In reply to Jean-Yves Avenard (On PTO until July 10) [:jya] from comment #50)
> Yes it would be good

Thanks Al and Jean-Yves. In that case, could you please nominate a patch for uplift to ESR45? Thanks!

Comment 53

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi guys, could you please provide an ESR45 patch for uplift as Jean-Yves is on PTO? Thanks!

Comment 54

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8760650 [details] [diff] [review]
[ffmpeg] Don't assume AVFrame has a constant size. esr45 patch

(In reply to Ritu Kothari (:ritu) from comment #53)
> Hi guys, could you please provide an ESR45 patch for uplift as Jean-Yves is
> on PTO? Thanks!

The patch was already there (and I've just checked, it's still applicable directly to ESR45), here's the nomination:


[Approval Request Comment] on behalf of :jya
If this is not a sec:{high,crit} bug, please state case for ESR consideration: sec-moderate, but it'd probably be good to uplift, "just in case" and to reduce crashes, and it's low risk.
User impact if declined: Crashes for lots of html5 videos, and small chance it could be exploitable.
Fix Landed on Version: 48, 49, 50.
Risk to taking this patch (and alternatives if risky): Low risk, it's replacing std memory ops with correct libav APIs where necessary.
String or UUID changes made by this patch: None.

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment 55

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8760650 [details] [diff] [review]
[ffmpeg] Don't assume AVFrame has a constant size. esr45 patch

Sec issue that seems safe to uplift to ESR45+

Comment 56

[Carsten Book [:Tomcat]]

 https://hg.mozilla.org/releases/mozilla-esr45/rev/d368e4ab89de

Comment 57

[Matt Wobensmith [:mwobensmith][:matt:]]

Hi Bert, can you verify that this has been fixed on your end?

Comment 58

[Bert Massop]

I'm away and unable to test for a few days. Will report ASAP after the 28th of this month.

Comment 59

[Bert Massop]

I cannot reproduce this bug on today's Nightly, nor on the 48.0b7 release from the beta channel. Looks fixed to me.