Closed Bug 479206 Opened 15 years ago Closed 15 years ago

Crash [@ construct_addresslist] when entering an invalid email address

Categories

(MailNews Core :: Composition, defect)

x86
macOS
defect
Not set
critical

Tracking

(Not tracked)

VERIFIED FIXED
Thunderbird 3.0b2

People

(Reporter: standard8, Assigned: standard8)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(2 files)

STR:

1) Compose an email
2) Set the address to:

test test@email>

3) Try to send the mail -> crashes

Stack:

#0  0x96d8fe70 in strlen ()
#1  0x96db2067 in strdup ()
#2  0x114f3196 in construct_addresslist (s=0x6135016 ">") at /Users/moztest/comm/main/src/mailnews/mime/src/comi18n.cpp:554
#3  0x114f4825 in apply_rfc2047_encoding (_src=0x6136050 "\"\"\"Moztest moztest\"@standard8.plus.com>\"", structured=1, charset=0x610ce60 "UTF-8", cursor=4, foldlen=72) at /Users/moztest/comm/main/src/mailnews/mime/src/comi18n.cpp:629
#4  0x114f4e35 in MIME_EncodeMimePartIIStr (header=0x6136050 "\"\"\"Moztest moztest\"@standard8.plus.com>\"", structured=1, mailCharset=0x610ce60 "UTF-8", fieldNameLen=4, encodedWordSize=72) at /Users/moztest/comm/main/src/mailnews/mime/src/comi18n.cpp:769
#5  0x114cbb9a in nsMimeConverter::EncodeMimePartIIStr_UTF8 (this=0x1d450c10, header=@0xbfffd714, structured=1, mailCharset=0x610ce60 "UTF-8", fieldnamelen=4, encodedWordSize=72, encodedString=0xbfffd674) at /Users/moztest/comm/main/src/mailnews/mime/src/nsMimeConverter.cpp:133
#6  0x1117768f in nsMsgI18NEncodeMimePartIIStr (header=0x6136050 "\"\"\"Moztest moztest\"@standard8.plus.com>\"", structured=1, charset=0x610ce60 "UTF-8", fieldnamelen=4, usemime=1) at /Users/moztest/comm/main/src/mailnews/base/util/nsMsgI18N.cpp:249
#7  0x112f90bf in mime_generate_headers (fields=0x610c900, charset=0x610ce60 "UTF-8", deliver_mode=0, aPrompt=0x4a87da0, status=0xbfffdce4) at /Users/moztest/comm/main/src/mailnews/compose/src/nsMsgCompUtils.cpp:606
#8  0x112d6ab4 in nsMsgComposeAndSend::GatherMimeAttachments (this=0x132a02d0) at /Users/moztest/comm/main/src/mailnews/compose/src/nsMsgSend.cpp:993
#9  0x11300832 in nsMsgAttachmentHandler::UrlExit (this=0x132a1cd0, status=0, aMsg=0x0) at /Users/moztest/comm/main/src/mailnews/compose/src/nsMsgAttachmentHandler.cpp:1350
#10 0x11300a38 in FetcherURLDoneCallback (aStatus=0, aContentType=@0x6116bfc, aCharset=@0x6116c08, totalSize=216, aMsg=0x0, tagData=0x132a1cd0) at /Users/moztest/comm/main/src/mailnews/compose/src/nsMsgAttachmentHandler.cpp:535
#11 0x113311e7 in nsURLFetcher::OnStopRequest (this=0x6116bb0, request=0x61376b0, ctxt=0x0, aStatus=0) at /Users/moztest/comm/main/src/mailnews/compose/src/nsURLFetcher.cpp:327
#12 0x14b4a824 in nsDocumentOpenInfo::OnStopRequest (this=0x612b490, request=0x61376b0, aCtxt=0x0, aStatus=0) at /Users/moztest/comm/main/src/mozilla/uriloader/base/nsURILoader.cpp:323
#13 0x11c2f69b in nsBaseChannel::OnStopRequest (this=0x6137680, request=0x612a750, ctxt=0x0, status=0) at /Users/moztest/comm/main/src/mozilla/netwerk/base/src/nsBaseChannel.cpp:680
#14 0x11c42804 in nsInputStreamPump::OnStateStop (this=0x612a750) at /Users/moztest/comm/main/src/mozilla/netwerk/base/src/nsInputStreamPump.cpp:576
#15 0x11c42924 in nsInputStreamPump::OnInputStreamReady (this=0x612a750, stream=0x610069c) at /Users/moztest/comm/main/src/mozilla/netwerk/base/src/nsInputStreamPump.cpp:401
#16 0x004df092 in nsInputStreamReadyEvent::Run (this=0x6128820) at /Users/moztest/comm/main/src/mozilla/xpcom/io/nsStreamUtils.cpp:111
#17 0x00511948 in nsThread::ProcessNextEvent (this=0x713bf0, mayWait=0, result=0xbfffe254) at /Users/moztest/comm/main/src/mozilla/xpcom/threads/nsThread.cpp:510
#18 0x0049ae0e in NS_ProcessPendingEvents_P (thread=0x713bf0, timeout=20) at nsThreadUtils.cpp:180
#19 0x120793a3 in nsBaseAppShell::NativeEventCallback (this=0x740710) at /Users/moztest/comm/main/src/mozilla/widget/src/xpwidgets/nsBaseAppShell.cpp:121
#20 0x120308f0 in nsAppShell::ProcessGeckoEvents (aInfo=0x740710) at /Users/moztest/comm/main/src/mozilla/widget/src/cocoa/nsAppShell.mm:374
#21 0x906235f5 in CFRunLoopRunSpecific ()
#22 0x90623cd8 in CFRunLoopRunInMode ()
#23 0x92d682c0 in RunCurrentEventLoopInMode ()
#24 0x92d68012 in ReceiveNextEventCommon ()
#25 0x92d67f4d in BlockUntilNextEventMatchingListInMode ()
#26 0x95e27d7d in _DPSNextEvent ()
#27 0x95e27630 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#28 0x95e2066b in -[NSApplication run] ()
#29 0x1202efea in nsAppShell::Run (this=0x740710) at /Users/moztest/comm/main/src/mozilla/widget/src/cocoa/nsAppShell.mm:693
#30 0x12e923b2 in nsAppStartup::Run (this=0x758e50) at /Users/moztest/comm/main/src/mozilla/toolkit/components/startup/src/nsAppStartup.cpp:192
#31 0x000b3d04 in XRE_main (argc=1, argv=0xbffff7d8, aAppData=0x70df70) at /Users/moztest/comm/main/src/mozilla/toolkit/xre/nsAppRunner.cpp:3279
#32 0x0000285f in main (argc=1, argv=0xbffff7d8) at /Users/moztest/comm/main/src/mail/app/nsMailApp.cpp:103

Not sure when it regressed, may be useful to know if it happens in b1.
Flags: blocking-thunderbird3?
Flags: blocking-seamonkey2.0a3?
My server rejects the address as invalid "5.1.3 User address required".  So dependent on server response?
Severity: normal → critical
(In reply to comment #1)
> My server rejects the address as invalid "5.1.3 User address required".  So
> dependent on server response?

I got something similar on Windows. Mac seems to be the only one crashing at the moment...
I actually tried on a self-built SeaMonkey from a few days ago (Gecko/20090216) on Linux and it "successfully" crashed.
FWIW I just reproduced this with beta 1.
This goes wayy back. I crash with both TB3 alpha 1 and alpha 2 using the steps in comment #0.

Time for zzz - else someone should test with TB 2.0.0.x...
(In reply to comment #5)
> Time for zzz - else someone should test with TB 2.0.0.x...

This does not crash with TB 2.0.0.19 -- official release mirrors download quickly but not for Mozilla FTP, another reason for me to go to bed.

The current regression window range is now (when 1.9 separated from 1.8.1.x) and TB3 alpha 1 release date.

Zzz..
Whoopie, regression window found. :)

does not crash in 20070611 Mac nightly.
crashes in 20070612 Mac nightly.

bonsai for regression range:

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=mozilla%2Fmail%2F+mozilla%2Fmailnews&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-06-11+02%3A00%3A00&maxdate=2007-06-12+04%3A00%3A00&cvsroot=%2Fcvsroot

Suspect: bug 379070 (Remove nsXPIDLCString / nsXPIDLString from mailnews) since it's the only one during that time period that touches compose, and anything containing the word "address" doesn't exist during that time period. Probably https://bugzilla.mozilla.org/show_bug.cgi?id=379070#c197 .

bienvenu, seems to be your sr on a patch by mscott.

(I hope my analysis is correct, feel free to set Depends on or Blocking as per needed)
Archived off a conversation in #maildev...

[19:55]    nth10sd> NeilAway: i hope i am right in my analysis though
[20:01]   NeilAway> nth10sd: well, 379070 could well be suspicious
[20:01]    nth10sd> true
[20:01]   NeilAway> nth10sd: since mime is on the stack...
[20:02]   NeilAway> nth10sd: chances are it's trying to strdup a null pointer
[20:03]   NeilAway> nth10sd: yeah, what happens is this
[20:03]   NeilAway> nth10sd: construct_addresslist looks for the "<" at the beginning of the address, and the ">" at the end
[20:04]   NeilAway> nth10sd: when it finds the ">" it tries to make a copy of the substring
[20:04]   NeilAway> nth10sd: this doesn't work out well when it never found a "<"
[20:04]   NeilAway> nth10sd: probably the old nsCRT::strdup (?) was null-safe

If MIME is part of the culprit, https://bugzilla.mozilla.org/show_bug.cgi?id=379070#c196 might be the real cause.. (I see nsCRT::strdup being removed and strdup somewhere being added in that patch)

I guess the QA work here will suffice for the moment. :)
The problem (at least on mac) is that these aren't being null-checked before strdup, therefore add checks, and deal appropriately. This fixes the problem for me on mac. I'll see if I can come up with a unit test later.
Assignee: nobody → bugzilla
Status: NEW → ASSIGNED
Attachment #363568 - Flags: superreview?(neil)
Attachment #363568 - Flags: review?(neil)
This same pattern (i.e, assuming that the string is non-null) is happening in other spots, see attachment 267283 [details] [diff] [review] from bug 379070.  Is there value in preemptively doing something similar for other cases of *strdup in another bug?
(In reply to comment #10)
> This same pattern (i.e, assuming that the string is non-null) is happening in
> other spots, see attachment 267283 [details] [diff] [review] from bug 379070.  Is there value in
> preemptively doing something similar for other cases of *strdup in another bug?

Or would making strdup null-safe be a better approach?
Attachment #363568 - Flags: superreview?(neil)
Attachment #363568 - Flags: superreview+
Attachment #363568 - Flags: review?(neil)
Attachment #363568 - Flags: review+
Attachment #363607 - Flags: review?(neil)
(In reply to comment #11)
> 
> Or would making strdup null-safe be a better approach?

No, strdup comes from the compiler/os c-library. One of the points to having a wrapper class or functions like nsCRT and PL_strdup, etc, was the ability to add null checking so part of the price of getting rid of those is losing that protection.
Attachment #363568 - Attachment description: The fix → [checked in] The fix
Attachment #363607 - Flags: review?(neil) → review+
Attachment #363607 - Attachment description: Testcase → [checked in] Testcase
Currently fixed for b3, do we want to fix for b2 as well?
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.0b3
Both SeaMonkey and Thunderbird have apparently shipped the previous milestones with that crash and didn't really have complaints, so I guess we can ship another one with it. That said, if we are doing a respin for something else, this might be a nice ride-along.
Landed on release branch:

changeset:   2048:3cd3c8d17db6
branch:      COMM_20090224_RELBRANCH
parent:      2044:7db1d7c5d187
user:        Mark Banner <bugzilla@standard8.plus.com>
date:        Mon Feb 23 14:31:11 2009 +0000
summary:     Testcase for Bug 479206 Crash [@ construct_addresslist] when entering an invalid email address. r=Neil

changeset:   2049:2617cb2e3af3
branch:      COMM_20090224_RELBRANCH
tag:         tip
user:        Mark Banner <bugzilla@standard8.plus.com>
date:        Sun Feb 22 22:21:26 2009 +0000
summary:     Bug 479206 Crash [@ construct_addresslist] when entering an invalid email address. r/sr=Neil
Flags: blocking-thunderbird3?
Flags: blocking-seamonkey2.0a3?
Target Milestone: Thunderbird 3.0b3 → Thunderbird 3.0b2
no sign of it in crash-stats
Status: RESOLVED → VERIFIED
Keywords: crash
Crash Signature: [@ construct_addresslist]
You need to log in before you can comment on or make changes to this bug.