Closed Bug 529253 Opened 16 years ago Closed 7 years ago

startup crash [@ nsJARChannel::OnStartRequest(nsIRequest*, nsISupports*) ] because nntp is violating AsyncOpen

Categories

(MailNews Core :: Networking: NNTP, defect)

Product:
MailNews Core
Component:
Networking: NNTP
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

(Not tracked)

Status:
RESOLVED WORKSFORME

People

(Reporter: vulnerable.zappa, Unassigned)

References

Details

(Keywords: crash, regression, testcase, Whiteboard: [sg:dos null-deref][startupcrash][patchlove][has all reviews, never checked in])

Crash Data

Attachments

(2 files)

testcase
93 bytes, text/html
Details
Fix double onStopRequest
1.26 KB, patch
Bienvenu
: review+
neil
: superreview+
Details | Diff | Splinter Review
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; pl; rv:1.9.1.5) Gecko/20091102 Firefox/3.5.5 Build Identifier: 2.0.0.23 We can remote crash Thunderbird and Seamonkey by usig jar:news://!/ url in parm of html tags Everthing is in poc http://exploits.offensive-security.com/record.php?id=9488 Reproducible: Always I wanted to write earlier about this bug But i did not have time Sorry
Contents of http://exploits.offensive-security.com/record.php?id=9488 for record keeping: ================================================================================ Mozilla Thunderbird 2.0.0.23 Mozilla Seamonkey 2.0 (jar50.dll) Null Pointer Derefernce (jar50.dll) Mozilla Thunderbird 2.0.0.23 Mozilla Seamonkey 2.0 eax=03d9b850 ebx=80000000 ecx=00000000 edx=00000000 esi=03d9b85c edi=80000000 eip=60016550 esp=0012d438 ebp=0012d5e4 iopl=0 nv up ei pl nz ac po cy cs=001b ss=0023 ds=0023 es=0023 fs=003b gs=0000 efl=00000217 funkcja: jar50 6001653a 5e pop esi 6001653b 8be5 mov esp,ebp 6001653d 5d pop ebp 6001653e c21800 ret 0x18 60016541 8b442404 mov eax,[esp+0x4] 60016545 56 push esi 60016546 8b5028 mov edx,[eax+0x28] 60016549 8b4824 mov ecx,[eax+0x24] 6001654c 83c0f4 add eax,0xfffffff4 6001654f 52 push edx 60016550 8b31 mov esi,[ecx] ; ds:0023:00000000=???????? 60016552 50 push eax 60016553 51 push ecx 60016554 ff560c call dword ptr [esi+0xc] ; 12 60016557 5e pop esi 60016558 c20c00 ret 0xc 6001655b 53 push ebx 6001655c 56 push esi 6001655d 8b74240c mov esi,[esp+0xc] 60016561 57 push edi 60016562 f6465780 test byte ptr [esi+0x57],0x80 Nie kochamy Kubusia Puchatka :) <img src="jar:news://!/">
0 seamonkey-bin nsJARChannel::OnStartRequest modules/libjar/nsJARChannel.cpp:864 1 seamonkey-bin nsJARChannel::OnDownloadComplete modules/libjar/nsJARChannel.cpp:848 2 seamonkey-bin nsDownloader::OnStopRequest netwerk/base/src/nsDownloader.cpp:163 3 seamonkey-bin nsNNTPProtocol::CleanupAfterRunningUrl mailnews/news/src/nsNNTPProtocol.cpp:5110 4 seamonkey-bin nsNNTPProtocol::CloseSocket mailnews/news/src/nsNNTPProtocol.cpp:5160 5 seamonkey-bin nsNNTPProtocol::CloseConnection mailnews/news/src/nsNNTPProtocol.cpp:5071 6 seamonkey-bin nsNNTPProtocol::LoadUrl mailnews/news/src/nsNNTPProtocol.cpp:1183 7 seamonkey-bin nsMsgProtocol::AsyncOpen mailnews/base/util/nsMsgProtocol.cpp:602 8 seamonkey-bin nsNNTPProtocol::AsyncOpen mailnews/news/src/nsNNTPProtocol.cpp:994 9 seamonkey-bin nsJARChannel::EnsureJarInput il.h:256 10 seamonkey-bin nsJARChannel::AsyncOpen modules/libjar/nsJARChannel.cpp:693 11 seamonkey-bin imgLoader::LoadImage modules/libpr0n/src/imgLoader.cpp:1278 12 seamonkey-bin nsContentUtils::LoadImage content/base/src/nsContentUtils.cpp:2421 13 seamonkey-bin nsImageLoadingContent::LoadImage content/base/src/nsImageLoadingContent.cpp:605 14 seamonkey-bin nsImageLoadingContent::LoadImage content/base/src/nsImageLoadingContent.cpp:509 15 seamonkey-bin nsHTMLImageElement::SetAttr content/html/content/src/nsHTMLImageElement.cpp:514 16 seamonkey-bin nsGenericHTMLElement::SetAttribute content/html/content/src/nsGenericHTMLElement.h:171 17 seamonkey-bin nsHTMLEditor::SetShadowPosition editor/libeditor/html/nsHTMLObjectResizer.cpp:803 18 seamonkey-bin nsHTMLEditor::ShowResizersInner editor/libeditor/html/nsHTMLObjectResizer.cpp:409 19 seamonkey-bin nsHTMLEditor::ShowResizers editor/libeditor/html/nsHTMLObjectResizer.cpp:338 20 seamonkey-bin nsHTMLEditor::CheckSelectionStateForAnonymousButtons editor/libeditor/html/nsHTMLAnonymousUtils.cpp:381 21 seamonkey-bin ResizerSelectionListener::NotifySelectionChanged editor/libeditor/html/nsHTMLObjectResizer.cpp:124 22 seamonkey-bin nsTypedSelection::NotifySelectionListeners layout/generic/nsSelection.cpp:6695 23 seamonkey-bin nsTypedSelection::Extend layout/generic/nsSelection.cpp:5993 24 seamonkey-bin nsFrameSelection::TakeFocus layout/generic/nsSelection.cpp:2003 25 seamonkey-bin nsFrame::HandlePress layout/generic/nsFrame.cpp:2062 26 seamonkey-bin nsFrame::HandleEvent layout/generic/nsFrame.cpp:1690 27 seamonkey-bin nsImageFrame::HandleEvent layout/generic/nsImageFrame.cpp:1488 28 seamonkey-bin nsPresShellEventCB::HandleEvent layout/base/nsPresShell.cpp:1375 29 seamonkey-bin nsEventTargetChainItem::HandleEventTargetChain content/events/src/nsEventDispatcher.cpp:346 30 seamonkey-bin nsEventDispatcher::Dispatch content/events/src/nsEventDispatcher.cpp:514 31 seamonkey-bin PresShell::HandleEventInternal layout/base/nsPresShell.cpp:6323 32 seamonkey-bin PresShell::HandlePositionedEvent layout/base/nsPresShell.cpp:6211 33 seamonkey-bin PresShell::HandleEvent layout/base/nsPresShell.cpp:6071 34 seamonkey-bin nsViewManager::HandleEvent view/src/nsViewManager.cpp:1400 35 seamonkey-bin nsViewManager::DispatchEvent view/src/nsViewManager.cpp:1359 36 seamonkey-bin HandleEvent view/src/nsView.cpp:168 37 seamonkey-bin nsChildView::DispatchEvent widget/src/cocoa/nsChildView.mm:2042 38 seamonkey-bin nsChildView::DispatchWindowEvent widget/src/cocoa/nsChildView.mm:2055 39 seamonkey-bin -[ChildView mouseDown:] widget/src/cocoa/nsChildView.mm:3661 40 AppKit -[NSWindow sendEvent:] 41 seamonkey-bin -[NSWindow nsCocoaWindow_NSWindow_sendEvent:] widget/src/cocoa/nsCocoaWindow.mm:2201 42 seamonkey-bin -[ToolbarWindow sendEvent:] widget/src/cocoa/nsCocoaWindow.mm:1958 43 AppKit -[NSApplication sendEvent:] 44 AppKit -[NSApplication run] 45 seamonkey-bin nsAppShell::Run widget/src/cocoa/nsAppShell.mm:759 46 seamonkey-bin nsAppStartup::Run toolkit/components/startup/src/nsAppStartup.cpp:193 47 seamonkey-bin XRE_main toolkit/xre/nsAppRunner.cpp:3321 48 seamonkey-bin main suite/app/nsSuiteApp.cpp:103 49 seamonkey-bin seamonkey-bin@0x1b01 50 seamonkey-bin seamonkey-bin@0x1a28 51 @0x2
Summary: Crash application → Crash [@nsJARChannel::OnStartRequest(nsIRequest*, nsISupports*) ]
Attached file testcase —
If you hit cancel on the dialog that comes up, you get a slightly different stack. bp-5ac0da0e-a959-464f-9664-1d0b22091117 0 seamonkey-bin nsDownloader::OnStopRequest netwerk/base/src/nsDownloader.cpp:163 1 seamonkey-bin nsNNTPProtocol::CleanupAfterRunningUrl mailnews/news/src/nsNNTPProtocol.cpp:5110 2 seamonkey-bin nsNNTPProtocol::CloseSocket mailnews/news/src/nsNNTPProtocol.cpp:5160 3 seamonkey-bin nsMsgProtocol::OnStopRequest mailnews/base/util/nsMsgProtocol.cpp:467 4 seamonkey-bin nsNNTPProtocol::OnStopRequest mailnews/news/src/nsNNTPProtocol.cpp:1268 5 seamonkey-bin nsInputStreamPump::OnStateStop netwerk/base/src/nsInputStreamPump.cpp:576 6 seamonkey-bin nsInputStreamPump::OnInputStreamReady netwerk/base/src/nsInputStreamPump.cpp:401 7 libxpcom_core.so nsInputStreamReadyEvent::Run xpcom/io/nsStreamUtils.cpp:111 8 libxpcom_core.so nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:521 9 libxpcom_core.so NS_ProcessNextEvent_P nsThreadUtils.cpp:227 10 seamonkey-bin nsBaseAppShell::Run widget/src/xpwidgets/nsBaseAppShell.cpp:170 11 seamonkey-bin nsAppStartup::Run toolkit/components/startup/src/nsAppStartup.cpp:193 12 seamonkey-bin XRE_main toolkit/xre/nsAppRunner.cpp:3321 13 seamonkey-bin main suite/app/nsSuiteApp.cpp:103 14 libc-2.10.1.so libc-2.10.1.so@0x16b55 15 seamonkey-bin qcms_transform_data_gray_out_lut 16 libc-2.10.1.so libc-2.10.1.so@0x16b55 17 libvorbisenc.so.2.0.3 libvorbisenc.so.2.0.3@0xcaf8d 18 ld-2.10.1.so ld-2.10.1.so@0xff3 19 ld-2.10.1.so ld-2.10.1.so@0xd325 20 ld-2.10.1.so ld-2.10.1.so@0x827 21 libc-2.10.1.so libc-2.10.1.so@0x1ff3 22 ld-2.10.1.so ld-2.10.1.so@0x12fbf 23 libc-2.10.1.so libc-2.10.1.so@0x16a7a 24 ld-2.10.1.so ld-2.10.1.so@0xff3
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: testcase
My bp-0e004e73-8859-47a8-a1db-8c2742091117 is basically the same thing as comment #2.
Severity: normal → critical
Component: General → Networking: NNTP
Keywords: crash
OS: Windows XP → All
Product: Thunderbird → MailNews Core
QA Contact: general → networking.nntp
Hardware: x86 → All
The common theme of those two stacks is that it's a call to mObserver/mListener. According to comment 0/comment 1, this is a null deref. Interesting. Investigating further...
nsJARChannel::OnStartRequest nsJARChannel::OnDownloadComplete nsDownloader::OnStopRequest nsNNTPProtocol::CleanupAfterRunningUrl ... nsNNTPProtocol::AsyncOpen nsJARChannel::AsyncOpen This is a bug in nntp violating AsyncOpen. It isn't allowed to trigger sync things. That said, I'll gladly fix this in JARChannel, i've tried to fix it that way before, but the necko team objected claiming "api violation" (which is correct).
So this is the stack I'm getting when pressing cancel: #0 0x00007fffe8490f9c in nsJARChannel::OnStartRequest (this=0x7fffd8e22380, req=0x0, ctx=0x0) at /usr/local/google/home/cbiesinger/comm-central/mozilla/modules/libjar/nsJARChannel.cpp:868 #1 0x00007fffe8492c51 in nsJARChannel::OnDownloadComplete (this=0x7fffd8e22380, downloader=0x7fffd7ed0a40, request=0x7fffd7e4ac20, context=0x0, status=2147500034, file=0x0) at /usr/local/google/home/cbiesinger/comm-central/mozilla/modules/libjar/nsJARChannel.cpp:852 #2 0x00007fffe91782f1 in nsDownloader::OnStopRequest (this=0x7fffd7ed0a40, request=0x7fffd7e4ac20, ctxt=0x0, status=2147500034) at /usr/local/google/home/cbiesinger/comm-central/mozilla/netwerk/base/src/nsDownloader.cpp:163 #3 0x00007fffdd8315f9 in nsNNTPProtocol::CleanupAfterRunningUrl (this=0x7fffd7e4ac00) at /usr/local/google/home/cbiesinger/comm-central/mailnews/news/src/nsNNTPProtocol.cpp:5117 #4 0x00007fffdd8318cb in nsNNTPProtocol::CloseSocket (this=0x7fffd7e4ac00) at /usr/local/google/home/cbiesinger/comm-central/mailnews/news/src/nsNNTPProtocol.cpp:5167 #5 0x00007fffdd82ffa6 in nsNNTPProtocol::CloseConnection (this=0x7fffd7e4ac00) at /usr/local/google/home/cbiesinger/comm-central/mailnews/news/src/nsNNTPProtocol.cpp:5078 #6 0x00007fffdd83cf2b in nsNNTPProtocol::LoadUrl (this=0x7fffd7e4ac00, aURL=0x7fffd7f18ec8, aConsumer=0x0) at /usr/local/google/home/cbiesinger/comm-central/mailnews/news/src/nsNNTPProtocol.cpp:1190 #7 0x00007fffeb112f39 in nsMsgProtocol::AsyncOpen (this=0x7fffd7e4ac18, listener=0x7fffd7ed0a40, ctxt=0x0) at /usr/local/google/home/cbiesinger/comm-central/mailnews/base/util/nsMsgProtocol.cpp:602 #8 0x00007fffdd83dba0 in nsNNTPProtocol::AsyncOpen (this=0x7fffd7e4ac00, listener=0x7fffd7ed0a40, ctxt=0x0) at /usr/local/google/home/cbiesinger/comm-central/mailnews/news/src/nsNNTPProtocol.cpp:1001 [...] It seems that nsNNTPChannel::AsyncOpen is not actually async.
Since I don't have time to learn nsNNTPChannel enough to fix this bug, I'll leave it to someone else to make asyncOpen actually async. FWIW, I'm not sure that this bug should be security bug. It's just a crash.
There is in fact a second problem where mailnews calls OnStopRequest twice (this is the case when you click OK on the "Subscribe?" dialog)
So this patch works (fixes the crash), but there's a scary comment in nsNNTPProtocol: // send StopRequest notification after we've cleaned up the protocol // because it can synchronously causes a new url to get run in the // protocol - truly evil, but we're stuck at the moment. if (m_channelListener) rv = m_channelListener->OnStopRequest(this, m_channelContext, NS_OK); So is this patch safe? This onStopRequest call would happen earlier than that... I think.
Attachment #412836 - Flags: review?(bienvenu)
What's essentially happening to cause the async problem is that canceling the subscribe dialog causes a synchronous, immediate connection close: if (confirmResult) ... else { ... return CloseConnection(); } It seems the right thing to do is to have that call fail without calling OnStopRequest (since OnStartRequest shouldn't have been called yet). Looking at CVS blame, that is what used to happen, but sspitzer changed that with bug 108256...
I can't say for sure - we'd need to try it out.
Whiteboard: [sg:dos null-deref]
Comment on attachment 412836 [details] [diff] [review] Fix double onStopRequest a wee bit scary, but seems like a reasonable thing to do.
Attachment #412836 - Flags: superreview?(neil)
Attachment #412836 - Flags: review?(bienvenu)
Attachment #412836 - Flags: review+
Attachment #412836 - Flags: superreview?(neil) → superreview+
Assignee: nobody → cbiesinger
Crash Signature: [@nsJARChannel::OnStartRequest(nsIRequest*, nsISupports*) ]
still needs checkin? pretty rare - 4 crashes in 4 months bp-a5d09e66-1c13-4252-a972-804252111006 is the only version 7
Crash Signature: [@nsJARChannel::OnStartRequest(nsIRequest*, nsISupports*) ] → [@ nsJARChannel::OnStartRequest(nsIRequest*, nsISupports*) ]
Summary: Crash [@nsJARChannel::OnStartRequest(nsIRequest*, nsISupports*) ] → Crash [@ nsJARChannel::OnStartRequest(nsIRequest*, nsISupports*) ]
I'm clearly not going to do anything with this bug anytime soon
Assignee: cbiesinger → nobody
Opening this up based on comment #9.
Group: core-security
has all reviews, so unclear why it never checked in. looks like somewhat increased crash rate when new releases come out. for example bp-123dc871-ee18-433f-89dc-b59142130308
Blocks: 108256
Whiteboard: [sg:dos null-deref] → [sg:dos null-deref][startupcrash][patchlove][never checked in]
we still have crashes [1] but nothing on crash-stats have nsNNTPProtocol on stack. so I think we should retest comment 0. If reproduces then suggest we just land this after we branch TB24. [1] bp-d1dc0dd3-c281-4efd-977d-ab10c2130601 0 xul.dll nsJARChannel::OnStartRequest modules/libjar/nsJARChannel.cpp:850 1 xul.dll nsInputStreamPump::OnStateStart netwerk/base/src/nsInputStreamPump.cpp:417 2 xul.dll nsInputStreamPump::OnInputStreamReady netwerk/base/src/nsInputStreamPump.cpp:368 3 xul.dll nsOutputStreamReadyEvent::Run xpcom/io/nsStreamUtils.cpp:82
Crash Signature: [@ nsJARChannel::OnStartRequest(nsIRequest*, nsISupports*) ] → [@ nsJARChannel::OnStartRequest(nsIRequest*, nsISupports*) ] [@ nsJARChannel::OnStartRequest]
Keywords: qawanted
Summary: Crash [@ nsJARChannel::OnStartRequest(nsIRequest*, nsISupports*) ] → startup crash [@ nsJARChannel::OnStartRequest(nsIRequest*, nsISupports*) ]
We're starting to see this crash on b2g : URL : https://marketplace.firefox.com/app/5bd02e34-8ccb-4a1f-8459-e000d78907da/manifest.webapp Signature Source 0 libxul.so nsJARChannel::OnStartRequest(nsIRequest*, nsISupports*) /home/geeksphone/FOS/peak/gecko/modules/libjar/nsJARChannel.cpp:1041 1 libxul.so nsInputStreamPump::OnStateStart() /home/geeksphone/FOS/peak/gecko/netwerk/base/src/nsInputStreamPump.cpp:521 2 libxul.so nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) /home/geeksphone/FOS/peak/gecko/netwerk/base/src/nsInputStreamPump.cpp:433 3 libxul.so nsInputStreamReadyEvent::Run() 4 libxul.so nsThread::ProcessNextEvent(bool, bool*) /home/geeksphone/FOS/peak/gecko/xpcom/threads/nsThread.cpp:770 5 libxul.so NS_ProcessNextEvent(nsIThread*, bool) /home/geeksphone/FOS/peak/gecko/xpcom/glue/nsThreadUtils.cpp:265 6 libxul.so nsThread::Shutdown() /home/geeksphone/FOS/peak/gecko/xpcom/threads/nsThread.cpp:599 7 libxul.so mozilla::AudioInitTask::Run() /home/geeksphone/FOS/peak/gecko/content/media/AudioStream.cpp:635 8 libxul.so nsThread::ProcessNextEvent(bool, bool*) /home/geeksphone/FOS/peak/gecko/xpcom/threads/nsThread.cpp:770 9 libxul.so NS_ProcessNextEvent(nsIThread*, bool) /home/geeksphone/FOS/peak/gecko/xpcom/glue/nsThreadUtils.cpp:265 10 libxul.so mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /home/geeksphone/FOS/peak/gecko/ipc/glue/MessagePump.cpp:99 11 libxul.so mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) /home/geeksphone/FOS/peak/gecko/ipc/glue/MessagePump.cpp:302 12 libxul.so MessageLoop::RunInternal() /home/geeksphone/FOS/peak/gecko/ipc/chromium/src/base/message_loop.cc:229 13 libxul.so MessageLoop::Run() /home/geeksphone/FOS/peak/gecko/ipc/chromium/src/base/message_loop.cc:222 14 libxul.so nsBaseAppShell::Run() /home/geeksphone/FOS/peak/gecko/widget/xpwidgets/nsBaseAppShell.cpp:164 15 libxul.so XRE_RunAppShell /home/geeksphone/FOS/peak/gecko/toolkit/xre/nsEmbedFunctions.cpp:693 16 libxul.so mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) /home/geeksphone/FOS/peak/gecko/ipc/glue/MessagePump.cpp:272 17 libxul.so MessageLoop::RunInternal() /home/geeksphone/FOS/peak/gecko/ipc/chromium/src/base/message_loop.cc:229 18 libxul.so MessageLoop::Run() /home/geeksphone/FOS/peak/gecko/ipc/chromium/src/base/message_loop.cc:222 19 libxul.so XRE_InitChildProcess /home/geeksphone/FOS/peak/gecko/toolkit/xre/nsEmbedFunctions.cpp:530 20 plugin-container main /home/geeksphone/FOS/peak/gecko/ipc/app/MozillaRuntimeMain.cpp:147 21 libc.so __libc_init /home/geeksphone/FOS/keon_nightly/bionic/libc/bionic/libc_init_dynamic.c:114 More reports: https://crash-stats.mozilla.com/report/list?product=B2G&signature=nsJARChannel%3A%3AOnStartRequest%28nsIRequest*%2C%20nsISupports*%29#tab-reports
status-b2g-v2.1: --- → affected
Whiteboard: [sg:dos null-deref][startupcrash][patchlove][never checked in] → [sg:dos null-deref][startupcrash][patchlove][never checked in][b2g-crash]
This bug has nothing to do with b2g. removing whiteboard, status-b2g
status-b2g-v2.1: affected → ---
Keywords: qawanted → regression
Summary: startup crash [@ nsJARChannel::OnStartRequest(nsIRequest*, nsISupports*) ] → startup crash [@ nsJARChannel::OnStartRequest(nsIRequest*, nsISupports*) ] because nntp is violating AsyncOpen
Whiteboard: [sg:dos null-deref][startupcrash][patchlove][never checked in][b2g-crash] → [sg:dos null-deref][startupcrash][patchlove][has all reviews, never checked in]
Removing myslef on all the bugs I'm cced on. Please NI me if you need something on MailNews Core bugs from me.
only 4 crashes, all 52.5.2. All startup. Like Bug 791149, no crashes since Feb 1, and no crashes in 52.6.0. Perhaps fixed by 1264673 which landed in 52.6.0. If anyone still sees a problem please update the bug report.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WORKSFORME
See Also: → 1264673
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: