The default bug view has changed. See this FAQ.

crash in nsSmtpProtocol when parsing IDN

RESOLVED FIXED in Thunderbird 23.0

Status

MailNews Core
Composition
--
critical
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: m_kato, Assigned: m_kato)

Tracking

({crash, regression})

Thunderbird 23.0
crash, regression

Thunderbird Tracking Flags

(thunderbird21+ fixed, thunderbird22+ fixed)

Details

(Whiteboard: [regression:TB20], crash signature)

Attachments

(2 attachments)

(Assignee)

Description

4 years ago
This is regression of bug 127399 (IDN support).  When lastAt is nullptr, the following crash occur due to invalid IDN?.

This bug was filed from the Socorro interface and is 
report bp-4c8b81bc-16e9-4c44-988e-874862130330 .
============================================================= 
0 	msvcr100.dll 	memcpy 	f:\dd\vctools\crt_bld\SELF_X86\crt\src\INTEL\memcpy.asm:185
1 	xul.dll 	nsACString_internal::Assign 	xpcom/string/src/nsTSubstring.cpp:314
2 	xul.dll 	nsACString_internal::Assign 	xpcom/string/src/nsTSubstring.cpp:290
3 	xul.dll 	nsACString_internal::Assign 	xpcom/string/src/nsTSubstring.cpp:308
4 	xul.dll 	nsACString_internal::Assign 	xpcom/string/src/nsTSubstring.cpp:290
5 	xul.dll 	nsSmtpProtocol::LoadUrl 	mailnews/compose/src/nsSmtpProtocol.cpp:1772
6 	xul.dll 	NS_MsgLoadSmtpUrl 	mailnews/compose/src/nsSmtpService.cpp:224
7 	xul.dll 	nsSmtpService::SendMailMessage 	mailnews/compose/src/nsSmtpService.cpp:105
8 	xul.dll 	nsMsgComposeAndSend::DeliverFileAsMail 	mailnews/compose/src/nsMsgSend.cpp:3640
9 	xul.dll 	nsMsgComposeAndSend::DeliverMessage 	mailnews/compose/src/nsMsgSend.cpp:3494
10 	xul.dll 	nsMsgComposeAndSend::GatherMimeAttachments 	mailnews/compose/src/nsMsgSend.cpp:1013
11 	xul.dll 	nsMsgAttachmentHandler::UrlExit 	mailnews/compose/src/nsMsgAttachmentHandler.cpp:1222
12 	xul.dll 	FetcherURLDoneCallback 	mailnews/compose/src/nsMsgAttachmentHandler.cpp:470
13 	xul.dll 	nsURLFetcher::OnStopRequest 	mailnews/compose/src/nsURLFetcher.cpp:281
14 	xul.dll 	nsDocumentOpenInfo::OnStopRequest 	uriloader/base/nsURILoader.cpp:297
15 	xul.dll 	nsBaseChannel::OnStopRequest 	netwerk/base/src/nsBaseChannel.cpp:737
16 	xul.dll 	nsInputStreamPump::OnStateStop 	netwerk/base/src/nsInputStreamPump.cpp:552
17 	xul.dll 	nsInputStreamPump::OnInputStreamReady 	netwerk/base/src/nsInputStreamPump.cpp:374
18 	xul.dll 	nsOutputStreamReadyEvent::Run 	xpcom/io/nsStreamUtils.cpp:82
19 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:627
20 	xul.dll 	NS_ProcessNextEvent_P 	objdir-tb/mozilla/xpcom/build/nsThreadUtils.cpp:238
21 	xul.dll 	mozilla::ipc::MessagePump::Run 	ipc/glue/MessagePump.cpp:82
22 	xul.dll 	MessageLoop::RunHandler 	ipc/chromium/src/base/message_loop.cc:208
23 	xul.dll 	MessageLoop::Run 	ipc/chromium/src/base/message_loop.cc:182
24 	xul.dll 	nsBaseAppShell::Run 	widget/xpwidgets/nsBaseAppShell.cpp:163
25 	xul.dll 	nsAppShell::Run 	widget/windows/nsAppShell.cpp:154
26 	xul.dll 	nsAppStartup::Run 	toolkit/components/startup/nsAppStartup.cpp:288
27 	xul.dll 	XREMain::XRE_mainRun 	toolkit/xre/nsAppRunner.cpp:3823
28 	xul.dll 	XREMain::XRE_main 	toolkit/xre/nsAppRunner.cpp:3890
29 	xul.dll 	XRE_main 	toolkit/xre/nsAppRunner.cpp:4093
30 	thunderbird.exe 	do_main 	mail/app/nsMailApp.cpp:111
31 	thunderbird.exe 	NS_internal_main 	mail/app/nsMailApp.cpp:200
32 	thunderbird.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:105
33 	thunderbird.exe 	__tmainCRTStartup 	crtexe.c:552
34 	kernel32.dll 	BaseThreadInitThunk 	
35 	ntdll.dll 	__RtlUserThreadStart 	
36 	ntdll.dll 	_RtlUserThreadStart
(Assignee)

Updated

4 years ago
Assignee: nobody → m_kato
Component: General → Composition
Product: Thunderbird → MailNews Core
(Assignee)

Updated

4 years ago
Blocks: 127399
(Assignee)

Comment 1

4 years ago
Created attachment 731761 [details] [diff] [review]
fix
(Assignee)

Updated

4 years ago
Attachment #731761 - Flags: review?(mbanner)
(Assignee)

Comment 2

4 years ago
Created attachment 731775 [details] [diff] [review]
test
Attachment #731775 - Flags: review?(mbanner)
Summary: crash in memcpy when parsing IDN → crash in nsSmtpProtocol when parsing IDN

Comment 3

4 years ago
So is the comment "// Fortunately, we will always have an @ in each mailbox address." in the code right? Does it need some update?

Updated

4 years ago
Keywords: regression
Version: Trunk → 20
tracking-thunderbird21: --- → +
tracking-thunderbird22: --- → +
(In reply to :aceman from comment #3)
> So is the comment "// Fortunately, we will always have an @ in each mailbox
> address." in the code right? Does it need some update?

So yes, that's wrong really. In theory we could have just a local address (no domain), but I'm not sure if that's still really used these days.

I think for now, the patch is probably fine (with the fix to the comment) to fix the crash, and we'll see if there's any issues when we push it out.
Comment on attachment 731761 [details] [diff] [review]
fix

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

::: mailnews/compose/src/nsSmtpProtocol.cpp
@@ -1768,3 @@
>            if (firstEvil)
>            {
>              // Fortunately, we will always have an @ in each mailbox address.

As aceman, says we should update, probably remove this comment, because it isn't true.
Attachment #731761 - Flags: review?(mbanner) → review+
Attachment #731775 - Flags: review?(mbanner) → review+
Comment on attachment 731761 [details] [diff] [review]
fix

[Triage Comment]
Due to the crashiness of this in TB 20 beta, and the fact we're just about to spin the one beta for this cycle, I've landed this on the beta branch:

https://hg.mozilla.org/releases/comm-beta/rev/1d1de403c584

We'll land on trunk and aurora once trunk re-opens.
Attachment #731761 - Flags: approval-comm-beta+
status-thunderbird21: --- → fixed
tracking-thunderbird23: --- → +

Comment 7

4 years ago
(In reply to Mark Banner (:standard8) from comment #4)
> (In reply to :aceman from comment #3)
> > So is the comment "// Fortunately, we will always have an @ in each mailbox
> > address." in the code right? Does it need some update?
> 
> So yes, that's wrong really.

Well, it wasn't when I wrote it; or rather: the codepath from clicking "Send" in the editor down here would have killed any other addresses. I'm a bit puzzled about what changed actually?

> In theory we could have just a local address (no domain),

Of course.

> but I'm not sure if that's still really used these days.

At least we didn't support it.
Which, otoh, I do loath sometimes, so I should not have relied on this assumption here.
Whiteboard: [regression:TB20]
https://hg.mozilla.org/comm-central/rev/96d7f4197146
https://hg.mozilla.org/releases/comm-aurora/rev/506abd6aa42d
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-thunderbird22: --- → fixed
tracking-thunderbird23: + → ---
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 23.0
You need to log in before you can comment on or make changes to this bug.