Closed Bug 504613 Opened 12 years ago Closed 11 years ago
crash (segfault) @ oggplay
_data _handle _theora _frame (memcpy) when playing corrupted ogg theora file
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:184.108.40.206) Gecko/2009060308 Ubuntu/9.04 (jaunty) Firefox/3.0.11 Build Identifier: mozilla-central revision c121e64e9940 Segmentation fault when playing corrupted ogg theora file (attached). Reproducible: Always Steps to Reproduce: 1. Load attached file. Actual Results: firefox crashes Expected Results: some sort of "this file is corrupted" message Backtrace from a program that uses the ogg libraries similarly to firefox (i.e. oggplayer): Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0xb6e69b90 (LWP 31797)] 0xb7b45896 in memcpy () from /lib/tls/i686/cmov/libc.so.6 (gdb) backtrace #0 0xb7b45896 in memcpy () from /lib/tls/i686/cmov/libc.so.6 #1 0x0930bfd8 in ?? () #2 0x08057449 in oggplay_callback_theora (oggz=0x929d200, op=0x930c128, serialno=855307237, user_data=0x930bfd8) at oggplay_callback.c:201 #3 0x0805fdfd in oggz_read_deliver_packet (elem=0x930c128) at oggz_read.c:303 #4 0x080649b8 in oggz_dlist_deliter (dlist=0x929d440, func=0x805fd00 <oggz_read_deliver_packet>) at oggz_dlist.c:183 #5 0x0806045f in oggz_read_sync (oggz=0x929d200) at oggz_read.c:462 #6 0x08060783 in oggz_read (oggz=0x929d200, n=8192) at oggz_read.c:606 #7 0x080555e6 in oggplay_step_decoding (me=0x929d040) at oggplay.c:691 #8 0x0804d04b in decode_thread (p=0xbfac0548) at oggplayer.cpp:313 #9 0xb7e2983b in ?? () from /usr/lib/libSDL-1.2.so.0 #10 0xb7e7771d in ?? () from /usr/lib/libSDL-1.2.so.0 #11 0xb7eb74ff in start_thread () from /lib/tls/i686/cmov/libpthread.so.0 #12 0xb7bb049e in clone () from /lib/tls/i686/cmov/libc.so.6
if the previous test case doesn't immediately crash firefox, try this one
This is a Read AV, exploitability would depend on what we're going to do with the data. If its just jumbled video that's probably OK, if we're reading it into a structure that might later be used as pointers or buffer sizes that isn't (assuming the attacker can control the junk you're copying from). eax=11fa1ba5 ebx=11fa16a5 ecx=00000140 edx=00000000 esi=11fa16a5 edi=1e200030 eip=7814838a esp=056ffd10 ebp=056ffd18 iopl=0 nv up ei pl nz ac po nc cs=001b ss=0023 ds=0023 es=0023 fs=003b gs=0000 efl=00010212 MOZCRT19!memcpy+0x5a: 7814838a f3a5 rep movs dword ptr es:[edi],dword ptr [esi] es:0023:1e200030=00000000 ds:0023:11fa16a5=???????? 0:020> !exploitable -v HostMachine\HostUser Executing Processor Architecture is x86 Debuggee is in User Mode Debuggee is a live user mode debugging session on the local machine Event Type: Exception Exception Faulting Address: 0x11fa16a5 First Chance Exception Type: STATUS_ACCESS_VIOLATION (0xC0000005) Exception Sub-Type: Read Access Violation Faulting Instruction:7814838a rep movs dword ptr es:[edi],dword ptr [esi] Exception Hash (Major/Minor): 0x6216665a.0x5b20632d Stack Trace: MOZCRT19!memcpy+0x5a xul!JSD_DebuggerOnForUser+0x3331 xul!JSD_DebuggerOn+0x4900 xul!NS_LogAddRef_P+0x4f24 xul!gfxFontTestStore::gfxFontTestStore+0x349f xul!gfxFontTestStore::gfxFontTestStore+0x3fc0 xul!gfxWindowsFontGroup::operator=+0xa57 xul!gfxContext::GetFlags+0xac6c Instruction Address: 0x7814838a Description: Read Access Violation on Block Data Move Short Description: ReadAVonBlockMove Exploitability Classification: PROBABLY_EXPLOITABLE Recommended Bug Title: Probably Exploitable - Read Access Violation on Block Data Move starting at MOZCRT19!memcpy+0x5a (Hash=0x6216665a.0x5b20632d) This is a read access violation in a block data move, and is therefore classified as probably exploitable.
Bug poke: is this being worked on?
11 years ago
Duplicate of this bug: 507415
11 years ago
Summary: crash (segfault) @oggplay_callback_theora when playing corrupted ogg theora file → crash (segfault) @ oggplay_data_handle_theora_frame (memcpy) when playing corrupted ogg theora file
So, this is a variant of the bug fixed in bug 500311. The first time we call theora_decode_packetin, th_decode_packetin returns OC_DUPFRAME. theora_decode_packetin ignores the result and returns 0. oggplay_callback_theora then calls theora_decode_YUVout, which returns an all-zeros (not sure if this is guaranteed, or if we're just reading uninitialized memory here) yuv_buffer. oggplay_data_handle_theora_frame then crashes trying to memcpy with a null source pointer.
In bug 500311, we crashed because the decoding resumed in a random place in the middle of the stream. Here, the stream is reset to 0 due to the call to oggplay_get_duration in LoadOggHeaders. Then, DecodeFrame calls oggplay_step_decoding, which resumes decoding from the first packet. The packet is identified as a header in oggplay_callback_theora and passed to theora_decode_header which returns OC_BADPACKET because the decoder has already been initialized. oggplay_callback_theora returns OGGZ_STOP_ERR, which eventually causes oggz_read to return OGGZ_ERR_STOP_ERR. oggplay_step_decoding ignores this (as it only checks for r == 0 and r == OGGZ_ERR_HOLE_IN_DATA), and calls oggz_read again. As the first three packets are passed to the decoder again, the first two are rejected by theora_decode_header and oggplay_callback_theora returns an error as described above. The third is not identified as a header by theora_packet_isheader (as it is a 0 byte packet, not the same third packet as the first time through), and is passed to theora_decode_packetin (and th_decode_packetin which returns OC_DUPFRAME, which is then swallowed by the caller and 0 is returned). Stream is theora only, packets are: -00:00:00.008: serialno 0855307237, granulepos 0|0, packetno 0 *** bos: 42 bytes -00:00:00.008: serialno 0855307237, calc. gpos 0|0, packetno 1: 47 bytes -00:00:00.008: serialno 0855307237, granulepos 0|0, packetno 2: 2.552 kB -00:00:00.008: serialno 0855307237, calc. gpos 0|0, packetno 4: 0 bytes Note that packetno 3 is missing. So: - should oggplay_step_decoding ignore that error? - where does the 0 byte third packet come from? are we processing packets 0, 1, and then 4 the second time?
cpearce mentioned that this might be fixed by the initialization changes in bug 512328 (update liboggplay), but it still crashes the same way. Curiously, the issue where packetno==2 is not presented to oggplay_callback_theora has gone away. Thinking about it, looking at the stream dump, and ignoring the error handling/packet reprocessing stuff... the first non-header packet that's going to be processed is packetno==4, which is zero bytes, and thus will be treated by Theora as a duplicate frame. Since there's no previous frame data to fall back on, we're going to get either an all-zeroes or uninitialized buffer. The old pre 1.0 Theora API also doesn't report the packet is a duplicate frame: ret=th_decode_packetin(api->decode,_op,&gp); if(ret<0)return OC_BADPACKET; _td->granulepos=gp; return 0; th_decode_packetin returns TH_DUPFRAME, but instead of the old API wrapper returning OC_DUPFRAME, it returns 0. So it might be difficult to detect this case correctly.
th_decode_packetin reports a duplicate frame when _op->bytes == 0. oggplay could perform the same check after calling theora_decode_packetin (it's necessary to call this so the internal decoder state is kept up to date). It also needs a flag in the OggPlayTheoraDecode instance to track when the first valid frame is seen, to deal with the case where there is a sequence of 0 byte packets before a valid frame is seen. I'm not sure if it makes sense to do this and try to play invalid files, or to reject them outright when we detect this case. If we try to play them, the attached file doesn't crash, but also doesn't fire an ended event (probably because there are no valid frames). Also, do we need to be careful to only call the decoder once we see the first valid *intra* frame? If so, we may have a similar bug when a stream begins with an inter frame...
Discussed this with Timothy Terriberry and Gregory Maxwell in #theora and determined that the Theora decoder should be treating these zero byte packets like other inter frames that are decoded before an intra frame has been seen. In this case, the decoder initializes the frame buffer with solid gray. Timothy has committed a fix to Theora SVN as revision r16557. Attached patch is a trivial backport of that to our in-tree Theora 1.0 source.
Assignee: nobody → kinetik
Status: NEW → ASSIGNED
Same patch, but with README_MOZILLA and update.sh updated, and with a crashtest included.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Flags: wanted1.9.2? → wanted1.9.2+
Attachment #400690 - Flags: approval1.9.2? → approval1.9.2+
Comment on attachment 400690 [details] [diff] [review] patch v0 Approved for 220.127.116.11, a=dveditz for release-drivers Should we upgrade Theora as we have done with other media libraries in 18.104.22.168, or is this one patch all we need?
Attachment #400690 - Flags: approval22.214.171.124? → approval126.96.36.199+
Pushed to 1.9.1: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/982600e5e62a
(In reply to comment #14) > (From update of attachment 400690 [details] [diff] [review]) > Approved for 188.8.131.52, a=dveditz for release-drivers > > Should we upgrade Theora as we have done with other media libraries in 184.108.40.206, > or is this one patch all we need? I think we don't need libtheora 1.1. It's a large patch, and we're a bit unsure about taking it because of that. We also don't think we need it in order to prevent these "crash with large frame size" bugs, as thanks to bug 504843's patch, we abort decoder initialization if we encounter excessively large theora frame sizes before initializing the frame decoder. Provided the decode of the header packets is safe (and it seems to be, we don't crash in the test files), we don't need the libtheora 1.1 update. dveditz: does that satisfy you? We can mark bug 515882 as fixed-1.9.1 as well, since the frame size integers can't be big enough to overflow thanks to bug 504843's patch.
That makes sense to me.
Verified fix using nightly 1.9.1 build for 11/9 using attached testcases.
You need to log in before you can comment on or make changes to this bug.