Closed
Bug 639303
Opened 13 years ago
Closed 13 years ago
FF4 crashes on multipart image/motion jpeg stream changing image size [@ @0x0 | mozilla::imagelib::Decoder::Finish() ]
Categories
(Core :: Graphics: ImageLib, defect)
Tracking
()
People
(Reporter: bugzilla, Assigned: MatsPalmgren_bugz)
References
()
Details
(Keywords: crash, reproducible, verified1.9.2, Whiteboard: [sg:critical][bug 638018 is fixing this on the 1.9.2 branch])
Crash Data
Attachments
(5 files)
|
12.08 KB,
text/plain
|
Details | |
|
6.13 KB,
text/plain
|
Details | |
|
11.69 KB,
patch
|
joe
:
review+
johnath
:
approval-mozilla-aurora+
dveditz
:
approval2.0+
|
Details | Diff | Splinter Review |
|
5.28 KB,
text/plain
|
Details | |
|
3.43 KB,
patch
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.15) Gecko/20110303 Firefox/3.6.15 Build Identifier: Nightly Build 5.March 2011 downloaded from http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/firefox-4.0b13pre.en-US.mac.dmg Web page containing UI elements (select box). Changing the select box element will remotely (on the server) start a CGI script and set the relevant parameter. Something crashes FF4. Reproducible: Always Steps to Reproduce: 1. Open URL http://appcam.mobotixcam.de:2080 and authorize. Authorization Credentials (type without the quotes!) user : "user" password: "Firefox" 2. From select box (the one in the middle) choose "Image Size" 3. From select box (the one on the right) choose "Mega" or "VGA", whatever is not selected Actual Results: FF4 crashes. Expected Results: Image size is set to the selected size.
|
Reporter | |
Updated•13 years ago
|
Version: unspecified → Trunk
|
|
Reporter | |
Comment 1•13 years ago
|
||
Crashes at least on Windows XP and Mac Os 10.6.x
OS: Mac OS X → All
|
||
Comment 2•13 years ago
|
||
Confirmed on: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b13pre) Gecko/20110303 Firefox/4.0b13pre bp-55cbacf9-17fb-4ee2-a162-952a32110306
Status: UNCONFIRMED → NEW
Component: General → ImageLib
Ever confirmed: true
Keywords: crash,
reproducible
Product: Firefox → Core
QA Contact: general → imagelib
Summary: FF4 crashes (involving: simple javascript remote call) → FF4 crashes (involving: simple javascript remote call) [@ @0x0 | mozilla::imagelib::Decoder::Finish() ]
|
|
Reporter | |
Comment 3•13 years ago
|
||
The problem might not be directly connected to using Javascript for simple remote scripting. It might by that changing the image size alone just confuses image rendering.
|
||
Comment 4•13 years ago
|
||
Confirmed on Mac OS X 10.5 with last night's nightly; it hung my browser for a really, really, really long time, then the browser eventually crashed with a SIGBUS. This is a readily-reproduceable crash. Nominating for blocking. This may also be a dupe of bug 623187. Before the crash, it's was hung on thread 1, in mozilla::imagelib::Decoder::PostDataError(): (gdb) thread apply all backtrace Thread 40 (process 4959 thread 0x16bcf): #0 0x973c22c6 in semaphore_timedwait_signal_trap () #1 0x973f42af in _pthread_cond_wait () #2 0x973f5b33 in pthread_cond_timedwait_relative_np () #3 0x96011c62 in TSWaitOnConditionTimedRelative () #4 0x96032a60 in MPWaitOnQueue () #5 0x95f6bdb4 in TFolderSizeTask::FolderSizeTaskProc () #6 0x96030fbb in PrivateMPEntryPoint () #7 0x973f3155 in _pthread_start () #8 0x973f3012 in thread_start () Thread 39 (process 4959 thread 0x16a03): #0 0x973c944e in __semwait_signal () #1 0x973f43e6 in _pthread_cond_wait () #2 0x973f3dcd in pthread_cond_wait$UNIX2003 () #3 0x00050db1 in PR_WaitCondVar () #4 0x00050e09 in PR_Wait () #5 0x03a69a9a in nsEventQueue::GetEvent () Cannot access memory at address 0x5 Thread 38 (process 4959 thread 0x1aa57): #0 0x973c944e in __semwait_signal () #1 0x973f43e6 in _pthread_cond_wait () #2 0x974199f8 in pthread_cond_timedwait$UNIX2003 () #3 0x0004ff77 in pt_TimedWait () #4 0x00050d17 in PR_WaitCondVar () #5 0x02bc9c5c in nsHostResolver::GetHostToLookup () Previous frame inner to this frame (gdb could not unwind past this frame) Thread 37 (process 4959 thread 0x12a2f): #0 0x973f29c6 in kevent () #1 0x93daae5f in __monitor_file_descriptor__ () #2 0x973f3155 in _pthread_start () #3 0x973f3012 in thread_start () Thread 36 (process 4959 thread 0x129f7): #0 0x973c944e in __semwait_signal () #1 0x973f43e6 in _pthread_cond_wait () #2 0x973f3dcd in pthread_cond_wait$UNIX2003 () #3 0x96032de3 in TSWaitOnCondition () #4 0x96011c36 in TSWaitOnConditionTimedRelative () #5 0x96032a60 in MPWaitOnQueue () #6 0x95f69e6a in TNodeSyncTask::SyncTaskProc () #7 0x96030fbb in PrivateMPEntryPoint () #8 0x973f3155 in _pthread_start () #9 0x973f3012 in thread_start () Thread 35 (process 4959 thread 0x1bbe3): #0 0x973c2266 in mach_msg_trap () #1 0x973c9a5c in mach_msg () #2 0x93dd5e7e in CFRunLoopRunSpecific () #3 0x93dd6b04 in CFRunLoopRun () #4 0x95f5ee88 in TFSEventsNotificationTask::FSEventsNotificationTaskProc () #5 0x96030fbb in PrivateMPEntryPoint () #6 0x973f3155 in _pthread_start () #7 0x973f3012 in thread_start () Thread 34 (process 4959 thread 0x1932b): #0 0x973c2266 in mach_msg_trap () #1 0x973c9a5c in mach_msg () #2 0x93dd5e7e in CFRunLoopRunSpecific () #3 0x93dd6b04 in CFRunLoopRun () #4 0x95f5ed0f in TSystemNotificationTask::SystemNotificationTaskProc () #5 0x96030fbb in PrivateMPEntryPoint () #6 0x973f3155 in _pthread_start () #7 0x973f3012 in thread_start () Thread 33 (process 4959 thread 0x6437): #0 0x973c944e in __semwait_signal () #1 0x973f43e6 in _pthread_cond_wait () #2 0x974199f8 in pthread_cond_timedwait$UNIX2003 () #3 0x0004ff77 in pt_TimedWait () #4 0x00050d17 in PR_WaitCondVar () #5 0x02bc9c5c in nsHostResolver::GetHostToLookup () Previous frame inner to this frame (gdb could not unwind past this frame) Thread 32 (process 4959 thread 0x1acc3): #0 0x973c22c6 in semaphore_timedwait_signal_trap () #1 0x973f42af in _pthread_cond_wait () #2 0x973f5b33 in pthread_cond_timedwait_relative_np () #3 0x23623f87 in unregister_ShockwaveFlash () #4 0x23512768 in dyld_stub_AECountItems () #5 0x236240ac in unregister_ShockwaveFlash () #6 0x236240f0 in unregister_ShockwaveFlash () #7 0x23624216 in unregister_ShockwaveFlash () #8 0x973f3155 in _pthread_start () #9 0x973f3012 in thread_start () Thread 31 (process 4959 thread 0x19c2b): #0 0x973c22c6 in semaphore_timedwait_signal_trap () #1 0x973f42af in _pthread_cond_wait () #2 0x973f5b33 in pthread_cond_timedwait_relative_np () #3 0x23623f87 in unregister_ShockwaveFlash () #4 0x233da01e in dyld_stub_AECountItems () #5 0x236240ac in unregister_ShockwaveFlash () #6 0x236240f0 in unregister_ShockwaveFlash () #7 0x23624216 in unregister_ShockwaveFlash () #8 0x973f3155 in _pthread_start () #9 0x973f3012 in thread_start () Thread 30 (process 4959 thread 0x12c17): #0 0x973c22c6 in semaphore_timedwait_signal_trap () #1 0x973f42af in _pthread_cond_wait () #2 0x973f5b33 in pthread_cond_timedwait_relative_np () #3 0x23623f87 in unregister_ShockwaveFlash () #4 0x23512768 in dyld_stub_AECountItems () #5 0x236240ac in unregister_ShockwaveFlash () #6 0x236240f0 in unregister_ShockwaveFlash () #7 0x23624216 in unregister_ShockwaveFlash () #8 0x973f3155 in _pthread_start () #9 0x973f3012 in thread_start () Thread 29 (process 4959 thread 0xd813): #0 0x973c22c6 in semaphore_timedwait_signal_trap () #1 0x973f42af in _pthread_cond_wait () #2 0x973f5b33 in pthread_cond_timedwait_relative_np () #3 0x23623f87 in unregister_ShockwaveFlash () #4 0x23512768 in dyld_stub_AECountItems () #5 0x236240ac in unregister_ShockwaveFlash () #6 0x236240f0 in unregister_ShockwaveFlash () #7 0x23624216 in unregister_ShockwaveFlash () #8 0x973f3155 in _pthread_start () #9 0x973f3012 in thread_start () Thread 28 (process 4959 thread 0x1c1f7): #0 0x973c22c6 in semaphore_timedwait_signal_trap () #1 0x973f42af in _pthread_cond_wait () #2 0x973f5b33 in pthread_cond_timedwait_relative_np () #3 0x23623f87 in unregister_ShockwaveFlash () #4 0x23512768 in dyld_stub_AECountItems () #5 0x236240ac in unregister_ShockwaveFlash () #6 0x236240f0 in unregister_ShockwaveFlash () #7 0x23624216 in unregister_ShockwaveFlash () #8 0x973f3155 in _pthread_start () #9 0x973f3012 in thread_start () Thread 27 (process 4959 thread 0x18a8f): #0 0x973c22ae in semaphore_wait_signal_trap () #1 0x973f42c6 in _pthread_cond_wait () #2 0x97439539 in pthread_cond_wait () #3 0x23623fbf in unregister_ShockwaveFlash () #4 0x232175af in dyld_stub_AECountItems () #5 0x236240ac in unregister_ShockwaveFlash () #6 0x236240f0 in unregister_ShockwaveFlash () #7 0x23624216 in unregister_ShockwaveFlash () #8 0x973f3155 in _pthread_start () #9 0x973f3012 in thread_start () Thread 26 (process 4959 thread 0x8283): #0 0x973c22ae in semaphore_wait_signal_trap () #1 0x973f42c6 in _pthread_cond_wait () #2 0x97439539 in pthread_cond_wait () #3 0x23623fbf in unregister_ShockwaveFlash () #4 0x232175af in dyld_stub_AECountItems () #5 0x236240ac in unregister_ShockwaveFlash () #6 0x236240f0 in unregister_ShockwaveFlash () #7 0x23624216 in unregister_ShockwaveFlash () #8 0x973f3155 in _pthread_start () #9 0x973f3012 in thread_start () Thread 25 (process 4959 thread 0x156a3): #0 0x973c22c6 in semaphore_timedwait_signal_trap () #1 0x973f42af in _pthread_cond_wait () #2 0x973f5b33 in pthread_cond_timedwait_relative_np () #3 0x936b9dbc in -[NSCondition waitUntilDate:] () #4 0x936b9bd0 in -[NSConditionLock lockWhenCondition:beforeDate:] () #5 0x936b9b35 in -[NSConditionLock lockWhenCondition:] () #6 0x96b436e8 in -[NSUIHeartBeat _heartBeatThread:] () #7 0x93673dfd in -[NSThread main] () #8 0x936739a4 in __NSThread__main__ () #9 0x973f3155 in _pthread_start () #10 0x973f3012 in thread_start () Thread 24 (process 4959 thread 0x15c03): #0 0x974116fa in select$DARWIN_EXTSN () #1 0x93de12ef in __CFSocketManager () #2 0x973f3155 in _pthread_start () #3 0x973f3012 in thread_start () Thread 23 (process 4959 thread 0x12583): #0 0x973c944e in __semwait_signal () #1 0x973f43e6 in _pthread_cond_wait () #2 0x973f3dcd in pthread_cond_wait$UNIX2003 () #3 0x00050db1 in PR_WaitCondVar () #4 0x00050e09 in PR_Wait () #5 0x03a69a9a in nsEventQueue::GetEvent () Cannot access memory at address 0x5 Thread 22 (process 4959 thread 0xd003): #0 0x973c2266 in mach_msg_trap () #1 0x973c9a5c in mach_msg () #2 0x93dd5e7e in CFRunLoopRunSpecific () #3 0x93dd6aa8 in CFRunLoopRunInMode () #4 0x95ac55f8 in HALRunLoop::OwnThread () #5 0x95ac5480 in CAPThread::Entry () #6 0x973f3155 in _pthread_start () #7 0x973f3012 in thread_start () Thread 21 (process 4959 thread 0xa00b): #0 0x973c944e in __semwait_signal () #1 0x973f43e6 in _pthread_cond_wait () #2 0x973f3dcd in pthread_cond_wait$UNIX2003 () #3 0x00050db1 in PR_WaitCondVar () #4 0x00050e09 in PR_Wait () #5 0x03a69a9a in nsEventQueue::GetEvent () Cannot access memory at address 0x5 Thread 20 (process 4959 thread 0x8e03): #0 0x973c944e in __semwait_signal () #1 0x973f43e6 in _pthread_cond_wait () #2 0x973f3dcd in pthread_cond_wait$UNIX2003 () #3 0x00050db1 in PR_WaitCondVar () #4 0x00050e09 in PR_Wait () #5 0x03a69a9a in nsEventQueue::GetEvent () Cannot access memory at address 0x5 Thread 19 (process 4959 thread 0x8b1b): #0 0x973c944e in __semwait_signal () #1 0x973f43e6 in _pthread_cond_wait () #2 0x973f3dcd in pthread_cond_wait$UNIX2003 () #3 0x00050db1 in PR_WaitCondVar () #4 0x00050e09 in PR_Wait () #5 0x03a69a9a in nsEventQueue::GetEvent () Cannot access memory at address 0x5 Thread 18 (process 4959 thread 0x892b): #0 0x973c944e in __semwait_signal () #1 0x973f43e6 in _pthread_cond_wait () #2 0x973f3dcd in pthread_cond_wait$UNIX2003 () #3 0x00050db1 in PR_WaitCondVar () #4 0x00050e09 in PR_Wait () #5 0x03a69a9a in nsEventQueue::GetEvent () Cannot access memory at address 0x5 Thread 17 (process 4959 thread 0x8a0f): #0 0x973c944e in __semwait_signal () #1 0x973f43e6 in _pthread_cond_wait () #2 0x973f3dcd in pthread_cond_wait$UNIX2003 () #3 0x00050db1 in PR_WaitCondVar () #4 0x00050e09 in PR_Wait () #5 0x03a69a9a in nsEventQueue::GetEvent () Cannot access memory at address 0x5 Thread 16 (process 4959 thread 0x8d0b): #0 0x973c944e in __semwait_signal () #1 0x973f43e6 in _pthread_cond_wait () #2 0x973f3dcd in pthread_cond_wait$UNIX2003 () #3 0x00050db1 in PR_WaitCondVar () #4 0x00050e09 in PR_Wait () #5 0x03a69a9a in nsEventQueue::GetEvent () Cannot access memory at address 0x5 Thread 15 (process 4959 thread 0x8303): #0 0x973c944e in __semwait_signal () #1 0x973f43e6 in _pthread_cond_wait () #2 0x973f3dcd in pthread_cond_wait$UNIX2003 () #3 0x00050db1 in PR_WaitCondVar () #4 0x00050e09 in PR_Wait () #5 0x03a69a9a in nsEventQueue::GetEvent () Cannot access memory at address 0x5 Thread 14 (process 4959 thread 0x7d03): #0 0x973c944e in __semwait_signal () #1 0x973f43e6 in _pthread_cond_wait () #2 0x973f3dcd in pthread_cond_wait$UNIX2003 () #3 0x00050db1 in PR_WaitCondVar () #4 0x00050e09 in PR_Wait () #5 0x03a69a9a in nsEventQueue::GetEvent () Cannot access memory at address 0x5 Thread 13 (process 4959 thread 0x6903): #0 0x973c944e in __semwait_signal () #1 0x973f43e6 in _pthread_cond_wait () #2 0x973f3dcd in pthread_cond_wait$UNIX2003 () #3 0x00050db1 in PR_WaitCondVar () #4 0x03721a83 in nsCertVerificationThread::Run () Thread 12 (process 4959 thread 0x6707): #0 0x973c944e in __semwait_signal () #1 0x973f43e6 in _pthread_cond_wait () #2 0x973f3dcd in pthread_cond_wait$UNIX2003 () #3 0x00050db1 in PR_WaitCondVar () #4 0x037202de in nsSSLThread::Run () Cannot access memory at address 0x9 Thread 11 (process 4959 thread 0x6803): #0 0x973c944e in __semwait_signal () #1 0x973f43e6 in _pthread_cond_wait () #2 0x973f3dcd in pthread_cond_wait$UNIX2003 () #3 0x00050db1 in PR_WaitCondVar () #4 0x00050e09 in PR_Wait () #5 0x03a69a9a in nsEventQueue::GetEvent () Cannot access memory at address 0x5 Thread 10 (process 4959 thread 0x590f): #0 0x973c944e in __semwait_signal () #1 0x973f43e6 in _pthread_cond_wait () #2 0x973f3dcd in pthread_cond_wait$UNIX2003 () #3 0x00050db1 in PR_WaitCondVar () #4 0x00050e09 in PR_Wait () #5 0x03a69a9a in nsEventQueue::GetEvent () Cannot access memory at address 0x5 Thread 9 (process 4959 thread 0x5103): #0 0x973c944e in __semwait_signal () #1 0x973f43e6 in _pthread_cond_wait () #2 0x973f3dcd in pthread_cond_wait$UNIX2003 () #3 0x947e9b32 in glvmDoWork () #4 0x973f3155 in _pthread_start () #5 0x973f3012 in thread_start () Thread 8 (process 4959 thread 0x3803): #0 0x973c944e in __semwait_signal () #1 0x973f43e6 in _pthread_cond_wait () #2 0x973f3dcd in pthread_cond_wait$UNIX2003 () #3 0x00050db1 in PR_WaitCondVar () #4 0x03576a57 in XPCJSRuntime::WatchdogMain () #5 0x973f3155 in _pthread_start () #6 0x973f3012 in thread_start () Thread 7 (process 4959 thread 0x3703): #0 0x973c944e in __semwait_signal () #1 0x973f43e6 in _pthread_cond_wait () #2 0x973f3dcd in pthread_cond_wait$UNIX2003 () #3 0x00050db1 in PR_WaitCondVar () #4 0x03c2d682 in js::GCHelperThread::threadLoop () #5 0x973f3155 in _pthread_start () #6 0x973f3012 in thread_start () Thread 6 (process 4959 thread 0x3603): #0 0x974116fa in select$DARWIN_EXTSN () #1 0x00057d6e in poll () #2 0x00052979 in PR_Poll () #3 0x02bbcbdd in nsSocketTransportService::Poll () Previous frame inner to this frame (gdb could not unwind past this frame) Thread 5 (process 4959 thread 0x3107): #0 0x973c944e in __semwait_signal () #1 0x973f43e6 in _pthread_cond_wait () #2 0x974199f8 in pthread_cond_timedwait$UNIX2003 () #3 0x0004ff77 in pt_TimedWait () #4 0x00050d17 in PR_WaitCondVar () #5 0x03a6fa23 in TimerThread::Run () Previous frame inner to this frame (gdb could not unwind past this frame) Thread 4 (process 4959 thread 0x200b): #0 0x973c944e in __semwait_signal () #1 0x973f43e6 in _pthread_cond_wait () #2 0x973f3dcd in pthread_cond_wait$UNIX2003 () #3 0x00050db1 in PR_WaitCondVar () #4 0x03a7c08b in nsCycleCollectorRunner::Run () Thread 3 (process 4959 thread 0x2103): #0 0x973f29c6 in kevent () #1 0x03aa5b90 in kq_dispatch () Previous frame inner to this frame (gdb could not unwind past this frame) Thread 2 (process 4959 thread 0x1a03): #0 0x973c2266 in mach_msg_trap () #1 0x973c9a5c in mach_msg () #2 0x02b85aab in google_breakpad::ExceptionHandler::WaitForMessage () #3 0x973f3012 in thread_start () Thread 1 (process 4959 thread 0x20b): #0 0x02cea4b4 in mozilla::imagelib::Decoder::PostDataError () #1 0x02d035bc in mozilla::imagelib::nsJPEGDecoder::WriteInternal () (gdb) info registers eax 0x14e 334 ecx 0xb0a41d2c -1331421908 edx 0x973c944e -1757637554 ebx 0x973f3ded -1757463059 esp 0xb0a41d2c 0xb0a41d2c ebp 0xb0a41da8 0xb0a41da8 esi 0xb0a42000 -1331421184 edi 0x21a4b874 564443252 eip 0x973c944e 0x973c944e <__semwait_signal+10> eflags 0x246 582 cs 0x7 7 ss 0x1f 31 ds 0x1f 31 es 0x1f 31 fs 0x1f 31 gs 0x37 55 (gdb) x /8i $pc 0x973c944e <__semwait_signal+10>: jae 0x973c945e <__semwait_signal+26> 0x973c9450 <__semwait_signal+12>: call 0x973c9455 <__semwait_signal+17> 0x973c9455 <__semwait_signal+17>: pop %edx 0x973c9456 <__semwait_signal+18>: mov 0x96cdc6b(%edx),%edx 0x973c945c <__semwait_signal+24>: jmp *%edx 0x973c945e <__semwait_signal+26>: ret 0x973c945f <asprintf>: push %ebp 0x973c9460 <asprintf+1>: mov %esp,%ebp Finally, I poked around in gdb for several minutes trying get more info (did not modify the running process), and decided to step through the binary - to see if it was looping or what. This is when it crashed: (gdb) thread 1 [Switching to thread 1 (process 4959 thread 0x20b)] 0x02cea4b4 in mozilla::imagelib::Decoder::PostDataError () (gdb) n Single stepping until exit from function _ZN7mozilla8imagelib7Decoder13PostDataErrorEv, which has no line number information. 0x02d035bc in mozilla::imagelib::nsJPEGDecoder::WriteInternal () (gdb) Single stepping until exit from function _ZN7mozilla8imagelib13nsJPEGDecoder13WriteInternalEPKcj, which has no line number information. warning: Got an error handling event: "Cannot access memory at address 0x5". (gdb) Single stepping until exit from function _ZN13nsCOMPtr_baseD2Ev, which has no line number information. warning: Got an error handling event: "Cannot access memory at address 0x5". (gdb) bt #0 0x02ceac93 in mozilla::imagelib::Decoder::Finish () #1 0x2240a130 in ?? () Cannot access memory at address 0x5 (gdb) n Single stepping until exit from function _ZN7mozilla8imagelib7Decoder6FinishEv, which has no line number information. Bus error Unfortunately, the crash reporter did not launch and I didn't get a core dump. My browser was built from http://hg.mozilla.org/mozilla-central/rev/e56ecd8b3a68
blocking2.0: --- → ?
|
Assignee | |
Comment 5•13 years ago
|
||
This looks like it could be the same underlying bug as in bug 638018. I can reproduce this crash in a trunk debug build on Linux which makes me happy, because bug 638018 takes hours to reproduce...
Assignee: nobody → matspal
Group: core-security
Whiteboard: [sg:critical?]
|
|
||
Comment 6•13 years ago
|
||
Note - if anybody is looking at my debugger output, I snagged the info at the bottom of the backtraces on the wrong thread. Sorry 'bout that.
|
|
Assignee | |
Comment 7•13 years ago
|
||
The PostSize() call in nsJPEGDecoder::WriteInternal() leads to an error because it's a multipart channel and the image has a different size. http://mxr.mozilla.org/mozilla-central/source/modules/libpr0n/src/RasterImage.cpp#907 The DoError() call on line 927 leads to ShutdownDecoder() which nulls out 'mDecoder'; this is the last ref so it's deleted.
|
|
Assignee | |
Comment 8•13 years ago
|
||
ShutdownDecoder() also leads to a "Improper call to JPEG library in state 202" error with this stack.
|
|
Assignee | |
Comment 9•13 years ago
|
||
This is likely exploitable.
Whiteboard: [sg:critical?] → [sg:critical]
|
|
Assignee | |
Comment 10•13 years ago
|
||
1. add a stack local strong ref on mDecoder before calling anything that can lead to an error 2. add a few missing null-checks on mDecoder 3. notify the decoder of the error in RasterImage::SetSize() so that it can avoid calling FinishInternal() (which would lead to the JPEG library error in stack2) 4. make RasterImage::SyncDecode() return an error code if there was an error (not necessary to fix the crash but it seems like the right thing to do) Try server results pending...
Attachment #517303 -
Flags: review?(bobbyholley+bmo)
|
||
Updated•13 years ago
|
Attachment #517303 -
Flags: review?(bobbyholley+bmo) → review?(joe)
|
|
Reporter | |
Comment 11•13 years ago
|
||
The problem is not using Javascript to change the image size. The problem is changing the image size while the server is delivering a stream of images to the browser.
|
|
Reporter | |
Updated•13 years ago
|
Summary: FF4 crashes (involving: simple javascript remote call) [@ @0x0 | mozilla::imagelib::Decoder::Finish() ] → FF4 crashes on multipart image/motion jpeg stream changing image size [@ @0x0 | mozilla::imagelib::Decoder::Finish() ]
|
||
Comment 12•13 years ago
|
||
It's really painful to minus this as we're so close to shipping here. Talking with devs in triage (Joe and others), the fix is too complicated to take at this time. .x'ing.
blocking2.0: ? → .x+
|
|
Reporter | |
Comment 13•13 years ago
|
||
Are you saying FF4 will be published without this bug being fixed? This will be really painful for some users as "Server Push" (content-type: multipart/x-mixed-replace) is a popular method for webcams to present a motion jpeg stream on a web page. I worry that webcam users will encounter this problem early and often.
|
||
Comment 14•13 years ago
|
||
> Are you saying FF4 will be published without this bug being fixed? Yes. > I worry that webcam users will encounter this problem early and often. Do webcams often change the image size in mid-stream?
|
|
Reporter | |
Comment 15•13 years ago
|
||
(In reply to comment #14) > > I worry that webcam users will encounter this problem early and often. > > Do webcams often change the image size in mid-stream? Is this a trick question? :-) Yes, they do - if users change either aspect ratio ("display mode") or size of the image. As you see from the webcam above, those parameters are easily accessible.
|
|
||
Comment 16•13 years ago
|
||
> Is this a trick question? :-)
No. This is the first time I see a multipart jpeg bug (and I've seen a number at this point) where the page included UI to modify the image size...|
|
||
Comment 17•13 years ago
|
||
In any case, you answered my question. "It depends" seems to be the answer.
|
|
Reporter | |
Comment 18•13 years ago
|
||
(In reply to comment #16) > No. This is the first time I see a multipart jpeg bug (and I've seen a number > at this point) where the page included UI to modify the image size... To trigger the bug, the image size has to change mid-stream. It is not relevant whether this is done through UI on the page where the stream is displayed, on another page or even through remote API calls (without UI e.g. using curl, wget).
|
||
Comment 19•13 years ago
|
||
Comment on attachment 517303 [details] [diff] [review] fix v1 I'm not terribly happy that we have to put more error handling in every decoder, but it's not the end of the world. >diff --git a/modules/libpr0n/src/RasterImage.cpp b/modules/libpr0n/src/RasterImage.cpp >@@ -2248,27 +2256,31 @@ RasterImage::WriteToDecoder(const char * >+ nsRefPtr<Decoder> kungFuDeathGrip = mDecoder; > mInDecoder = PR_TRUE; > mDecoder->Write(aBuffer, aCount); > mInDecoder = PR_FALSE; > > // We unlock the current frame, even if that frame is different from the > // frame we entered the decoder with. (See above.) > if (mFrames.Length() > 0) { > imgFrame *curframe = mFrames.ElementAt(mFrames.Length() - 1); > curframe->UnlockImageData(); > } > >+ if (!mDecoder) >+ return NS_ERROR_FAILURE; >+ This is impossible - we hold a strong reference, and we even dereference mDecoder above. Just drop this null check. > CONTAINER_ENSURE_SUCCESS(mDecoder->GetDecoderError()); > > // Keep track of the total number of bytes written over the lifetime of the > // decoder > mBytesDecoded += aCount; > > return NS_OK; > } >@@ -2406,28 +2418,29 @@ RasterImage::SyncDecode() > rv = WriteToDecoder(mSourceData.Elements() + mBytesDecoded, > mSourceData.Length() - mBytesDecoded); > CONTAINER_ENSURE_SUCCESS(rv); > > // When we're doing a sync decode, we want to get as much information from the > // image as possible. We've send the decoder all of our data, so now's a good > // time to flush any invalidations (in case we don't have all the data and what > // we got left us mid-frame). >+ nsRefPtr<Decoder> kungFuDeathGrip = mDecoder; > mInDecoder = PR_TRUE; > mDecoder->FlushInvalidations(); > mInDecoder = PR_FALSE; > > // If we finished the decode, shutdown the decoder >- if (IsDecodeFinished()) { >+ if (mDecoder && IsDecodeFinished()) { > rv = ShutdownDecoder(eShutdownIntent_Done); > CONTAINER_ENSURE_SUCCESS(rv); > } Don't need this null check. >@@ -2688,17 +2703,17 @@ imgDecodeWorker::Run() > // flush once the whole frame is ready. > if (!image->mHasBeenDecoded) { > image->mInDecoder = PR_TRUE; > image->mDecoder->FlushInvalidations(); > image->mInDecoder = PR_FALSE; > } > > // If the decode finished, shutdown the decoder >- if (image->IsDecodeFinished()) { >+ if (image->mDecoder && image->IsDecodeFinished()) { And I'm pretty sure we don't need this null check either.
Attachment #517303 -
Flags: review?(joe) → review+
|
||
Comment 20•13 years ago
|
||
Mats: is this ready to land now? I'd like to see how it does on trunk soon so we can consider taking the fix on the mozilla2.0 branch for FF4.0.1. And since there are no nightly users on that branch we need all the mozilla-central testing we can get.
|
||
Comment 21•13 years ago
|
||
If this bug don't crash firefox 3.6.X , i don't think this patch is apropriate for Bug638018.
|
|
||
Comment 22•13 years ago
|
||
i Can't reproduce this , the password isn't "Firefox". A new password?
|
|
||
Comment 23•13 years ago
|
||
I can reproduce with an other testcase in 4.0 , i will try in 3.6.
|
|
Assignee | |
Comment 24•13 years ago
|
||
(In reply to comment #19) > >@@ -2248,27 +2256,31 @@ RasterImage::WriteToDecoder(const char * > > >+ nsRefPtr<Decoder> kungFuDeathGrip = mDecoder; > > mInDecoder = PR_TRUE; > > mDecoder->Write(aBuffer, aCount); > > mInDecoder = PR_FALSE; > > > > // frame we entered the decoder with. (See above.) > > if (mFrames.Length() > 0) { > > imgFrame *curframe = mFrames.ElementAt(mFrames.Length() - 1); > > curframe->UnlockImageData(); > > } > > > >+ if (!mDecoder) > >+ return NS_ERROR_FAILURE; > >+ > > This is impossible - we hold a strong reference, and we even dereference > mDecoder above. Just drop this null check. Just because we take a strong ref on the object mDecoder refers to doesn't prevent the mDecoder member from later being set to null. RasterImage::ShutdownDecoder does set mDecoder to null and it can be invoked while still having RasterImage methods on the stack that use mDecoder, like the method above. There's a path from the Write call above to ShutdownDecoder in the first stack I attached. I'd like to investigate these null-checks further because I think they are necessary. Unfortunately the server doesn't accept the login anymore. Jordi, do you have a alternative testcase I can use?
|
|
||
Comment 25•13 years ago
|
||
http://94.84.185.56/control/userimage.html
|
|
Reporter | |
Comment 26•13 years ago
|
||
(In reply to comment #24) > Unfortunately the server doesn't accept the login anymore. I reactivated the login (credentials as before) to the server: http://appcam.mobotixcam.de:2080/ Please complain if login fails.
|
|
Assignee | |
Comment 27•13 years ago
|
||
Some of the possible paths to RasterImage::ShutdownDecoder:
RasterImage::SourceDataComplete
imgRequest::OnStopRequest
RasterImage::RequestDecode
RasterImage::WantDecodedFrames
imgRequest::OnDataAvailable
imgRequest::RequestDecode
RasterImage::SyncDecode
RasterImage::Draw
RasterImage::ExtractFrame
RasterImage::CopyFrame
RasterImage::GetFrame
RasterImage::DoError
CONTAINER_ENSURE_SUCCESS
RasterImage::Init
RasterImage::DecodingComplete
RasterImage::AddSourceData
RasterImage::NewSourceData
RasterImage::InitDecoder
RasterImage::WriteToDecoder
RasterImage::DecodeSomeData
RasterImage::UnlockImage
CONTAINER_ENSURE_TRUE
RasterImage::GetImgFrame
RasterImage::GetDrawableImgFrame
RasterImage::GetCurrentDrawableImgFrame
RasterImage::GetCurrentImgFrame
RasterImage::GetCurrentFrameIsOpaque
RasterImage::GetCurrentFrameRect
RasterImage::StartAnimation
RasterImage::ResetAnimation
Image::EvaluateAnimation
Image::SetAnimationMode
Image::DecrementAnimationConsumers
Image::IncrementAnimationConsumers
RasterImage::InternalAddFrame
RasterImage::AppendPalettedFrame
RasterImage::Notify
RasterImage::EnsureCleanFrame
RasterImage::FrameUpdated
Decoder::FlushInvalidations
RasterImage::SetFrameDisposalMethod
RasterImage::SetFrameTimeout
RasterImage::SetFrameBlendMethod
RasterImage::SetFrameHasNoAlpha
RasterImage::SetSize
Decoder::PostSize
nsPNGDecoder::WriteInternal
Decoder::Write
RasterImage::WriteToDecoder
nsPNGDecoder::info_callback
imgDecodeWorker::Run
I can probably find more to add to that list, but as you can see most of the
RasterImage methods are already tainted.|
|
Assignee | |
Comment 28•13 years ago
|
||
Crash if I remove the null check I added in WriteToDecoder. (using http://appcam.mobotixcam.de:2080/)
|
|
Assignee | |
Comment 29•13 years ago
|
||
Comment on attachment 517303 [details] [diff] [review] fix v1 Please re-review -- I want to keep the null-checks I added, see additional information above why they are needed.
Attachment #517303 -
Flags: review+ → review?(joe)
|
|
Assignee | |
Updated•13 years ago
|
Whiteboard: [sg:critical] → [sg:critical][needs review]
|
|
Assignee | |
Comment 30•13 years ago
|
||
multipart/x-mixed-replace jpeg image crash test
|
|
||
Comment 31•13 years ago
|
||
Alright. I can buy those null checks, but perhaps not where they are put. Shouldn't we null-guard mDecoder everywhere we might use it when it's null?
|
|
Assignee | |
Comment 32•13 years ago
|
||
> Shouldn't we null-guard mDecoder everywhere we might use it when it's null?
Yes, but it's more important to land this soon to bake it for inclusion
on the stable branches to get the exploitable part fixed. We can worry
about null-pointer crashes later.
Do you have any specific null-checks in mind at this time that you want
to add or move?|
|
||
Comment 33•13 years ago
|
||
We're not going to hold up Macaw for this one, but we still want it on m-c and will look at future releases. Would this bug also apply to the 1.9.2 branch? That's where the crash was originally seen.
blocking2.0: ? → ---
status1.9.2:
--- → wanted
|
||
Comment 34•13 years ago
|
||
Joe, Mats, can we please get this reviewed and landed, or get a new patch that's landable soon?
|
||
Comment 35•13 years ago
|
||
Similar crashes with deleted memory seen on the following urls: http://511cam3.sccgov.org/santaclara/Traffic.shtml?name=Middlefield_WB_at_Oregon%26route=Oregon%26host=511cam3.sccgov.org%26fancy=Middlefield%2520WB%2520%40%2520Oregon http://511cam3.sccgov.org/santaclara/Traffic.shtml?name=Zanker_SB_at_Montague%26route=Montague%26host=511cam3.sccgov.org%26fancy=Zanker%2520SB%2520%40%2520Montague http://tracker.sjtu.edu.cn/ctest/safari.html
|
|
||
Comment 36•13 years ago
|
||
Comment on attachment 517303 [details] [diff] [review] fix v1 >@@ -2406,28 +2418,29 @@ RasterImage::SyncDecode() >- if (IsDecodeFinished()) { >+ if (mDecoder && IsDecodeFinished()) { > rv = ShutdownDecoder(eShutdownIntent_Done); > CONTAINER_ENSURE_SUCCESS(rv); > } >@@ -2688,17 +2703,17 @@ imgDecodeWorker::Run() > // If the decode finished, shutdown the decoder >- if (image->IsDecodeFinished()) { >+ if (image->mDecoder && image->IsDecodeFinished()) { These two null checks should be moved inside IsDecodeFinished or ShutdownDecoder, or wherever they're necessary. I'm OK with it if you do that as a followup bug.
Attachment #517303 -
Flags: review?(joe) → review+
|
|
Assignee | |
Comment 37•13 years ago
|
||
This should land on mozilla-shadow right? How do I check in there?
|
|
||
Updated•13 years ago
|
blocking1.9.2: --- → ?
blocking2.0: --- → ?
status-firefox5:
--- → affected
tracking-firefox5:
--- → ?
tracking-firefox6:
--- → ?
Whiteboard: [sg:critical][needs review] → [sg:critical]
|
|
||
Comment 39•13 years ago
|
||
Please land on mozilla-central. After some sanity checking we'll want it on mozilla-aurora and mozilla-1.9.2 (but please request approval for those branches)
|
|
||
Comment 40•13 years ago
|
||
Comment on attachment 517303 [details] [diff] [review] fix v1 We probably want this on aurora once it's baked on trunk a little while.
Attachment #517303 -
Flags: approval-mozilla-aurora?
|
||
Updated•13 years ago
|
Attachment #517303 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
|
|
||
Comment 41•13 years ago
|
||
Mats, can you land this?
|
|
||
Updated•13 years ago
|
Whiteboard: [sg:critical] → [sg:critical][needs trunk landing, branch patches]
|
||
Comment 42•13 years ago
|
||
Please land on central and aurora. the shadow repo is not quite ready.
|
|
||
Comment 43•13 years ago
|
||
Mats, do we need to reassign this bug to someone who will land the patch and respond to bugmail?
|
|
||
Comment 44•13 years ago
|
||
(In reply to comment #43) > Mats, do we need to reassign this bug to someone who will land the patch and > respond to bugmail? Sorry, that came out sounding wrong. But we really do need to land this.
|
|
Assignee | |
Comment 45•13 years ago
|
||
Sorry, I'm behind on reading bugmail, I'll try to land this later tonight...
|
|
Assignee | |
Comment 46•13 years ago
|
||
Landed on m-c, holding the testcase until the bug is public. Filed bug 656351 to followup on comment 36. http://hg.mozilla.org/mozilla-central/rev/86849e5fb7ca
Status: NEW → RESOLVED
Closed: 13 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Whiteboard: [sg:critical][needs trunk landing, branch patches] → [sg:critical][needs branch patches]
Target Milestone: --- → mozilla6
|
||
Comment 47•13 years ago
|
||
Please land for Aurora by Monday May 16 or the approval will potentially be lost. Please mark as status-firefox5 fixed when you do.
|
|
Assignee | |
Comment 48•13 years ago
|
||
http://hg.mozilla.org/releases/mozilla-aurora/rev/2c4a0ac34b33
|
|
||
Updated•13 years ago
|
Blocks: CVE-2011-2377
|
||
Comment 50•13 years ago
|
||
Last crash on Aurora in this signature showing as 20110427143820, so far the fix looks good from a crash stats point of view.
|
||
Comment 51•13 years ago
|
||
Per security group discussion, requesting landing on mozilla-2.0.
|
|
Assignee | |
Updated•13 years ago
|
Attachment #517303 -
Flags: approval2.0?
|
|
||
Updated•13 years ago
|
status-firefox6:
--- → fixed
|
|
||
Comment 52•13 years ago
|
||
Comment on attachment 517303 [details] [diff] [review] fix v1 Approved for the mozilla2.0 repository, a=dveditz for release-drivers
Attachment #517303 -
Flags: approval2.0? → approval2.0+
|
|
Assignee | |
Comment 53•13 years ago
|
||
http://hg.mozilla.org/releases/mozilla-2.0/rev/f77eaecf9e00
|
|
Assignee | |
Updated•13 years ago
|
Whiteboard: [sg:critical][needs branch patches] → [sg:critical][bug 638018 is fixing this on the 1.9.2 branch]
|
|
||
Updated•13 years ago
|
|
||
Updated•13 years ago
|
Crash Signature: [@ @0x0 | mozilla::imagelib::Decoder::Finish() ]
|
||
Comment 54•13 years ago
|
||
Verified for 1.9.2.18 with bug 638018. The testcase here doesn't crash in 1.9.2.
Keywords: verified1.9.2
|
|
||
Updated•13 years ago
|
Group: core-security
|
||
Comment 55•13 years ago
|
||
Mozilla/5.0 (X11; Linux i686; rv:6.0) Gecko/20100101 Firefox/6.0 Can you please provide another link (besides this one: http://appcam.mobotixcam.de:2080/) with which I can verify this issue? Thanks!
|
|
Reporter | |
Comment 56•13 years ago
|
||
(In reply to comment #55) > Can you please provide another link (besides this one: > http://appcam.mobotixcam.de:2080/) with which I can verify this issue? I will try to make the link working again. But this will not happen before next week.
|
|
Reporter | |
Comment 57•13 years ago
|
||
(In reply to comment #55) > Can you please provide another link (besides this one: > http://appcam.mobotixcam.de:2080/) with which I can verify this issue? The link is working again.
|
||
Comment 58•13 years ago
|
||
Setting resolution to Verified Fixed on Mozilla/5.0 (Windows NT 6.1; rv:6.0) Gecko/20100101 Firefox/6.0 beta 5 I have followed the STR and I've got no crash.
Status: RESOLVED → VERIFIED
|
|
Reporter | |
Comment 59•11 years ago
|
||
The link is permanently down.
You need to log in
before you can comment on or make changes to this bug.
Description
•