Last Comment Bug 722924 - Buffer overflow when decoding Ogg Vorbis
: Buffer overflow when decoding Ogg Vorbis
Status: VERIFIED FIXED
[sg:dos][qa!]
: crash, testcase
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla13
Assigned To: Timothy B. Terriberry (:derf)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-31 16:23 PST by [On PTO until 6/29]
Modified: 2012-10-18 12:23 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
verified
verified
wontfix
wontfix


Attachments
POC from comment 0 (556 bytes, application/octet-stream)
2012-01-31 16:23 PST, [On PTO until 6/29]
no flags Details
Fix handling of floor0 codebook with no used entries (123 bytes, patch)
2012-01-31 17:48 PST, Timothy B. Terriberry (:derf)
no flags Details | Diff | Review
Fix handling of Vorbis floor0 codebook with no used entries (2.03 KB, patch)
2012-01-31 17:51 PST, Timothy B. Terriberry (:derf)
kinetik: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
Details | Diff | Review

Description [On PTO until 6/29] 2012-01-31 16:23:31 PST
Created attachment 593265 [details]
POC from comment 0

Security researcher regenrecht contacted us to report a DOS Ogg Vorbis issue that was not addressed when we fixed his issue in bug 719612.

E-mail as follows:

Hi,

Here is just a litte addition (rather silly DoS) to the Vorbis issue.
From media/libvorbis/lib/vorbis_codebook.c:

long vorbis_book_decodev_set(codebook *book,float *a,oggpack_buffer
*b,int n){
  ...
    for(i=0;i<n;){
      for (j=0;j<book->dim;)
        a[i++]=0.f;
  ...
}

Heap buffer is being overflown uncontrollably with 0s until that final
iteration when program hits the end of mapped memory region.

Tested under 32-bit Firefox 9.0.1 Linux/Windows XP SP3:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0xaf076b40 (LWP 1254)]
0xb58624bd in vorbis_book_decodev_set (book=0xa7ed0340, a=0xaefffa60,
b=0xa8a11e50, n=1) at media/libvorbis/lib/vorbis_codebook.c:451
451	        a[i++]=0.f;
(gdb) p/x a+i
$1 = 0xaf000000
(gdb) x/i $pc
=> 0xb58624bd <vorbis_book_decodev_set+192>:	mov    %edx,(%eax)
(gdb) i r eax
eax            0xaf000000	-1358954496
(gdb) info proc mappings
...
	0xae4ff000 0xae500000     0x1000          0
	0xae500000 0xaf000000   0xb00000          0
	0xaf056000 0xaf057000     0x1000          0
...

(568.b4): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
eax=00000001 ebx=0429c1b0 ecx=00018f94 edx=00990040 esi=03be2140
edi=00000000
eip=014ed21c esp=068dfcd8 ebp=068dfce4 iopl=0         nv up ei pl nz na
po nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000
efl=00010202
xul!vorbis_book_decodev_set+0x5e:
014ed21c d91c8b          fstp    dword ptr [ebx+ecx*4]
ds:0023:04300000=????????
0:027> k
ChildEBP RetAddr
068dfce4 014ed9ef xul!vorbis_book_decodev_set+0x5e
[media\libvorbis\lib\vorbis_codebook.c @ 451]
068dfd10 015c3079 xul!floor0_inverse1+0xb4
[media\libvorbis\lib\vorbis_floor0.c @ 181]
068dfd9c 01477157 xul!mapping0_inverse+0xc2
[media\libvorbis\lib\vorbis_mapping0.c @ 726]
068dfdc0 015939c8 xul!vorbis_synthesis+0x190
[media\libvorbis\lib\vorbis_synthesis.c @ 89]
068dfe18 01911611 xul!nsOggReader::DecodeVorbis+0x21
[content\media\ogg\nsoggreader.cpp @ 348]
068dfe30 01553372 xul!nsOggReader::DecodeAudioData+0x40
[content\media\ogg\nsoggreader.cpp @ 409]
068dfe48 01553450
xul!nsBuiltinDecoderReader::DecodeToFirstData<AudioData>+0x4d
[content\media\nsbuiltindecoderreader.cpp @ 258]
068dfe74 015535ce xul!nsBuiltinDecoderReader::FindStartTime+0x5d
[content\media\nsbuiltindecoderreader.cpp @ 233]
068dfe94 019b84b7 xul!nsBuiltinDecoderStateMachine::FindStartTime+0x37
[content\media\nsbuiltindecoderstatemachine.cpp @ 1804]
068dfed4 019b8629 xul!nsBuiltinDecoderStateMachine::DecodeMetadata+0xaa
[content\media\nsbuiltindecoderstatemachine.cpp @ 1230]
068dfef0 0140d075 xul!nsBuiltinDecoderStateMachine::DecodeThreadRun+0x29
[content\media\nsbuiltindecoderstatemachine.cpp @ 305]
068dfef4 010ca55a xul!nsRunnableMethodImpl<void (__thiscall
nsMediaDecoder::*)(void),1>::Run+0xe
[obj-firefox\dist\include\nsthreadutils.h @ 346]
068dff24 0101e038 xul!nsThread::ProcessNextEvent+0x15a
[xpcom\threads\nsthread.cpp @ 637]
068dff44 10002470 xul!nsThread::ThreadFunc+0x88
[xpcom\threads\nsthread.cpp @ 272]
00000000 00000000 nspr4!_PR_NativeRunThread+0x120
[nsprpub\pr\src\threads\combined\pruthr.c @ 448]
0:027> g
(568.478): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
eax=04523fd0 ebx=0012ccc0 ecx=04230800 edx=00000000 esi=020aa2a0
edi=042d6840
eip=0113f67b esp=0012cb38 ebp=0012cb78 iopl=0         nv up ei pl nz na
po nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000
efl=00010202
xul!PresShell::HandleEventInternal+0x2b:
0113f67b 8b4204          mov     eax,dword ptr [edx+4]
ds:0023:00000004=????????
0:000> k
ChildEBP RetAddr
0012cb78 010146de xul!PresShell::HandleEventInternal+0x2b
[layout\base\nspresshell.cpp @ 6258]
0012cbcc 0114329d xul!PresShell::HandleEvent+0x31e
[layout\base\nspresshell.cpp @ 6045]
0012cbf4 0105ed5d xul!nsViewManager::HandleEvent+0x38
[view\src\nsviewmanager.cpp @ 1032]
0012cc54 01041cce xul!nsViewManager::DispatchEvent+0x2cd
[view\src\nsviewmanager.cpp @ 1007]
0012cc74 00ff8646 xul!AttachedHandleEvent+0x4e [view\src\nsview.cpp @ 192]
0012cc88 00ff8702 xul!nsWindow::DispatchEvent+0x36
[widget\src\windows\nswindow.cpp @ 3580]
0012cc9c 011fb36b xul!nsWindow::DispatchWindowEvent+0x13
[widget\src\windows\nswindow.cpp @ 3604]
0012ccf8 01213cdc xul!nsWindow::DispatchFocus+0x63
[widget\src\windows\nswindow.cpp @ 4134]
0012cd14 01025232 xul!nsWindow::DispatchFocusToTopLevelWindow+0x6e
[widget\src\windows\nswindow.cpp @ 4101]
0012cdf4 0103d057 xul!nsWindow::ProcessMessage+0x352
[widget\src\windows\nswindow.cpp @ 5234]
0012ce20 01016cab xul!nsWindow::WindowProcInternal+0xe7
[widget\src\windows\nswindow.cpp @ 4436]
0012ce64 01016c76 xul!CallWindowProcCrashProtected+0x2e
[xpcom\base\nscrashonexception.cpp @ 65]
0012ce80 7e368734 xul!nsWindow::WindowProc+0x1b
[widget\src\windows\nswindow.cpp @ 4378]
0012ceac 7e368816 USER32!InternalCallWinProc+0x28
0012cf14 7e378ea0 USER32!UserCallWinProcCheckWow+0x150
0012cf68 7e378eec USER32!DispatchClientMessage+0xa3
0012cf90 7c90e473 USER32!__fnDWORD+0x24
0012cfb4 7e3693e9 ntdll!KiUserCallbackDispatcher+0x13
0012cfe0 7e369402 USER32!NtUserPeekMessage+0xc
0012d00c 010e69c8 USER32!PeekMessageW+0xbc
0012d0b8 010e686b xul!nsAppShell::ProcessNextNativeEvent+0x88
[widget\src\windows\nsappshell.cpp @ 339]
0012d0d8 010ca4af xul!nsBaseAppShell::OnProcessNextEvent+0x19b
[widget\src\xpwidgets\nsbaseappshell.cpp @ 324]
0012d10c 10002a62 xul!nsThread::ProcessNextEvent+0xaf
[xpcom\threads\nsthread.cpp @ 598]
0012d154 01236112 nspr4!_MD_CURRENT_THREAD+0x12
[nsprpub\pr\src\md\windows\w95thred.c @ 310]
00000000 00000000 xul!MessageLoop::RunHandler+0x21
[ipc\chromium\src\base\message_loop.cc @ 202]
0:000> g
(568.b4): Access violation - code c0000005 (!!! second chance !!!)
eax=00000001 ebx=0429c1b0 ecx=00018f94 edx=00990040 esi=03be2140
edi=00000000
eip=014ed21c esp=068dfcd8 ebp=068dfce4 iopl=0         nv up ei pl nz na
po nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000
efl=00000202
xul!vorbis_book_decodev_set+0x5e:
014ed21c d91c8b          fstp    dword ptr [ebx+ecx*4]
ds:0023:04300000=????????
0:027> g
eax=000000c0 ebx=00000000 ecx=01d9a840 edx=0012a114 esi=7c97e440
edi=7c97e460
eip=7c90e514 esp=0605ff70 ebp=0605ffb4 iopl=0         nv up ei ng nz na
pe nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000
efl=00000286
ntdll!KiFastSystemCallRet:
7c90e514 c3              ret
Comment 1 [On PTO until 6/29] 2012-01-31 16:26:30 PST
I have verified the POC as causing a crash on OS X 10.7 with the nightly build: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:12.0a1) Gecko/20120131 Firefox/12.0a1.

Crash report is at https://crash-stats.mozilla.com/report/index/bp-15016879-70a6-45dd-ba8d-1f56c2120201.
Comment 2 Timothy B. Terriberry (:derf) 2012-01-31 17:48:37 PST
Created attachment 593278 [details] [diff] [review]
Fix handling of floor0 codebook with no used entries

This one's just a straight-up, simple bug. Fix attached.
Comment 3 Timothy B. Terriberry (:derf) 2012-01-31 17:51:42 PST
Created attachment 593280 [details] [diff] [review]
Fix handling of Vorbis floor0 codebook with no used entries

This time after remembering to run hg qref.
Comment 4 [On PTO until 6/29] 2012-01-31 17:58:26 PST
Crashes in Aurora (Fx 11) as well: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:11.0a2) Gecko/20120131 Firefox/11.0a2.
Comment 5 Huzaifa Sidhpurwala 2012-02-02 22:13:26 PST
Writing anything to heap buffer should still be considered a security issue, though not critical, its definitely not DoS
Comment 6 Huzaifa Sidhpurwala 2012-02-05 19:16:26 PST
This is now committed upstream
https://trac.xiph.org/changeset/18183

Is a CVE id assigned to this issue, or would it be done on the release date as usual?
Comment 7 Daniel Veditz [:dveditz] 2012-02-06 13:15:27 PST
We don't assign CVEs to client DoS bugs. How is this going to do anything except write until it triggers an access violation?
Comment 8 [On PTO until 6/29] 2012-02-07 09:31:49 PST
(In reply to Timothy B. Terriberry (:derf) from comment #3)
> Created attachment 593280 [details] [diff] [review]
> Fix handling of Vorbis floor0 codebook with no used entries
> 
> This time after remembering to run hg qref.

Are we going to check this in?
Comment 9 Timothy B. Terriberry (:derf) 2012-02-13 14:12:34 PST
Comment on attachment 593280 [details] [diff] [review]
Fix handling of Vorbis floor0 codebook with no used entries

[Approval Request Comment]
Regression caused by (bug #): Not a regression.
User impact if declined: Malicious content can cause a browser crash.
Testing completed (on m-c, etc.): Landed on inbound, passes try.
Risk to taking this patch (and alternatives if risky): Very low. One-line change, affected code doesn't even run for normal Vorbis files.
String changes made by this patch: None.
Comment 10 Timothy B. Terriberry (:derf) 2012-02-13 14:55:04 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/01b07f54ec50
Comment 11 Timothy B. Terriberry (:derf) 2012-02-14 06:09:01 PST
https://hg.mozilla.org/mozilla-central/rev/01b07f54ec50
Comment 12 Alex Keybl [:akeybl] 2012-02-21 14:42:24 PST
Comment on attachment 593280 [details] [diff] [review]
Fix handling of Vorbis floor0 codebook with no used entries

[Triage Comment]
Approved for Aurora 12 and Beta 11. Low-risk change that would only affect OGG.
Comment 14 Virgil Dicu [:virgil] [QA] 2012-03-07 01:51:05 PST
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (Windows NT 6.1; rv:11.0) Gecko/20100101 Firefox/11.0

Verified on Firefox 11 Beta 6 with the POC from comment 0. None of the files (html and ogg) crashes Firefox on Mac OS 10.7, Windows 7 and Ubuntu 11.10.

Could previously reproduce on F10
Comment 15 Virgil Dicu [:virgil] [QA] 2012-03-30 05:36:02 PDT
Mozilla/5.0 (X11; Linux i686; rv:12.0) Gecko/20100101 Firefox/12.0
Mozilla/5.0 (X11; Linux i686; rv:13.0) Gecko/20120330 Firefox/13.0a2
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:12.0) Gecko/20100101 Firefox/12.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:13.0) Gecko/20120323 Firefox/13.0a2
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0) Gecko/20100101 Firefox/12.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120329 Firefox/13.0a2

Marking as verified with Firefox 12 beta 3 and Firefox 13 on the same platforms as in comment 14.
Comment 16 [On PTO until 6/29] 2012-03-30 11:37:20 PDT
Technically, you should only mark a bug as "verified" if you verify it on trunk aka mozilla-central unless the bug is opened as a branch only bug. 

Otherwise, we use the status-firefoxXX flags.

I've verified this on trunk.
Comment 17 Virgil Dicu [:virgil] [QA] 2012-04-02 00:05:39 PDT
Al, I've marked it as verified as the issue was checked on all the affected Firefox versions (the versions where the patch has landed). In this case, verified in 11, 12, 13. 

If I remember well, this was discussed while Firefox 9 reached beta. This should be discussed again if there is a need in verifying a bug across Nightly, though in this case that would mean verifying more than 4 Firefox versions (if the merge is made before verifying the bug in Nightly).

https://wiki.mozilla.org/Releases/Firefox_12/Test_Plan#Bug_Fix_Verifications_3 (softvision verification etherpad)
Comment 18 [On PTO until 6/29] 2012-04-02 12:50:15 PDT
Virgil, *if* a bug affects nightly (as well as branches), the status of the bug as a whole reflects the trunk status, not that of branches. 

In other words, it isn't enough to check on branches and verify there if it also affects trunk if you want to verify the bug as fixed. You can use the status-firefoxXX flags for branches. The bug should stay open, if it affects nightly, until it is verified there.

QA doesn't generally verify security fixes on nightly (I've asked but they've said they don't have manpower) so I try to do so.

Note You need to log in before you can comment on or make changes to this bug.