Closed Bug 1571813 Opened 5 years ago Closed 5 years ago

Crash in [@ InvalidArrayIndex_CRASH | nsMsgComposeAndSend::HackAttachments]

Categories

(MailNews Core :: Composition, defect)

x86
Windows 10
defect
Not set
critical

Tracking

(thunderbird_esr68 unaffected, thunderbird69 fixed, thunderbird70 fixed)

RESOLVED FIXED
Thunderbird 70.0
Tracking Status
thunderbird_esr68 --- unaffected
thunderbird69 --- fixed
thunderbird70 --- fixed

People

(Reporter: wsmwk, Assigned: aceman)

References

(Regression)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

First crash report is 69.0b1

bp-a7e952ad-823b-45d9-8f29-9cc320190806.

Top 10 frames of crashing thread:

0 xul.dll InvalidArrayIndex_CRASH xpcom/ds/nsTArray.cpp:27
1 xul.dll nsMsgComposeAndSend::HackAttachments comm/mailnews/compose/src/nsMsgSend.cpp
2 xul.dll nsMsgComposeAndSend::Init comm/mailnews/compose/src/nsMsgSend.cpp:2793
3 xul.dll nsMsgComposeAndSend::CreateAndSendMessage comm/mailnews/compose/src/nsMsgSend.cpp:3715
4 xul.dll nsMsgCompose::SendMsgToServer comm/mailnews/compose/src/nsMsgCompose.cpp:1175
5 xul.dll nsMsgCompose::SendMsg comm/mailnews/compose/src/nsMsgCompose.cpp:1367
6 xul.dll NS_InvokeByIndex 
7 xul.dll XPCWrappedNative::CallMethod js/xpconnect/src/XPCWrappedNative.cpp:1158
8 xul.dll XPC_WN_CallMethod js/xpconnect/src/XPCWrappedNativeJSOps.cpp:943
9 xul.dll js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:540

Another one that doesn't make much sense.

comm/mailnews/compose/src/nsMsgCompose.cpp:1175 calls the following with attachments==null and preloaded_attachments==null
comm/mailnews/compose/src/nsMsgSend.cpp:3715 nsMsgComposeAndSend::CreateAndSendMessage calls
comm/mailnews/compose/src/nsMsgSend.cpp:2793 nsMsgComposeAndSend::Init which calls
HackAttachments. And that checks:
https://searchfox.org/comm-central/rev/2d079399f2abf8119b05fbd6bb5782b28412d881/mailnews/compose/src/nsMsgSend.cpp#1996

Also a bit hard to see why it would crash on entering that function :-(

This can be reproduced by sending a HTML reply to a news message with contains an embedded image. See bug 1572864 comment #56 for details.

Depends on: 1572864

Not really related to bug 1572864. Only when working in that one, I found a way to make it crash:

Go to m.t.multimedia on news.mozilla.org, find a message with an embedded image in the thread "test attachment", produce a HTML reply and send it, you'll crash.

Simple as that.

No longer depends on: 1572864

This is the stack for me:

xul.dll!InvalidArrayIndex_CRASH(unsigned __int64 aIndex, unsigned __int64 aLength) Line 27	C++
xul.dll!nsMsgComposeAndSend::HackAttachments(nsIArray * attachments, nsIArray * preloadedAttachments) Line 0	C++
xul.dll!nsMsgComposeAndSend::Init(nsIMsgIdentity * aUserIdentity, const char * aAccountKey, nsMsgCompFields * fields, nsIFile * sendFile, bool digest_p, bool dont_deliver_p, int mode, nsIMsgDBHdr * msgToReplace, const char * attachment1_type, const nsTSubstring<char> & attachment1_body, nsIArray * attachments, nsIArray * preloaded_attachments, const nsTSubstring<char16_t> & password, const nsTSubstring<char> & aOriginalMsgURI, int aType) Line 2793	C++
xul.dll!nsMsgComposeAndSend::CreateAndSendMessage(nsIEditor * aEditor, nsIMsgIdentity * aUserIdentity, const char * aAccountKey, nsIMsgCompFields * fields, bool mode, bool msgToReplace, int attachment1_type, nsIMsgDBHdr * attachment1_body, const char * attachments, const nsTSubstring<char> & preloaded_attachments, nsIArray * parentWindow, nsIArray * progress, mozIDOMWindowProxy * aListener, nsIMsgProgress * password, nsIMsgSendListener * aOriginalMsgURI, const nsTSubstring<char16_t> & aType, const nsTSubstring<char> &) Line 3715	C++
xul.dll!nsMsgCompose::SendMsgToServer(int deliverMode, nsIMsgIdentity * identity, const char * accountKey) Line 1186	C++
xul.dll!nsMsgCompose::SendMsg(int deliverMode, nsIMsgIdentity * identity, const char * accountKey, nsIMsgWindow * aMsgWindow, nsIMsgProgress * progress) Line 1378	C++

So line 2793 is this:
https://searchfox.org/comm-central/rev/2d079399f2abf8119b05fbd6bb5782b28412d881/mailnews/compose/src/nsMsgSend.cpp#2793
which is the call to HackAttachments(attachments, preloaded_attachments); with two null arguments.

Sadly line 0 isn't helpful. That due to the deadly mix of clang-cl compiler with MS VC as debugger :-(

Aceman tells me that I messed up here:
https://hg.mozilla.org/comm-central/rev/323c1caa59c5#l21.25

We need to fix that in TB 69 also.

Regressed by: 1557829
Attached patch 1571813.patch (deleted) — Splinter Review

It actually crashes at line 2232, assigning to member [0] of array AutoTArray<nsString, 1>, which does not seem to exist. It seems the array has 0 member on declaration and new members need to be added via .AppendElement first, not assigned directly (previously with char16_t *[1] that was possible).
Comes from bug 1557829.

There is also another case in nsAbView that we need to find STRs to get a crash to see if the fix helps.

Assignee: nobody → acelists
Status: NEW → ASSIGNED
Attachment #9086832 - Flags: review?(jorgk)

For me the relevant stack was this:
#8 0x00007fe4825e1063 in InvalidArrayIndex_CRASH(unsigned long, unsigned long) (aIndex=aIndex@entry=0, aLength=aLength@entry=0) at mozilla/xpcom/ds/nsTArray.cpp:27
#9 0x00007fe482350fc0 in nsTArray_Impl<nsTString<char16_t>, nsTArrayInfallibleAllocator>::ElementAt(unsigned long) (aIndex=0, this=0x7fffb2bcfa38) at mozilla/obj-x86_64-pc-linux-gnu/dist/include/nsTArray.h:347
#10 0x00007fe482350fc0 in nsTArray_Impl<nsTString<char16_t>, nsTArrayInfallibleAllocator>::operator[](unsigned long) (aIndex=0, this=0x7fffb2bcfa38) at mozilla/obj-x86_64-pc-linux-gnu/dist/include/nsTArray.h:1099
#11 0x00007fe482350fc0 in nsMsgComposeAndSend::HackAttachments(nsIArray*, nsIArray*) (this=this@entry=0x7fe460c22900, attachments=attachments@entry=0x0, preloadedAttachments=preloadedAttachments@entry=0x0)
at mozilla/comm/mailnews/compose/src/nsMsgSend.cpp:2232

Using operator[] on element 0 of a 0 length array...

Comment on attachment 9086832 [details] [diff] [review] 1571813.patch Fantastic. Great job finding that, but I told you: You're debugger is better than mine ;-) - Finally I can send news messages with images as HTML (not a common use case).
Attachment #9086832 - Flags: review?(jorgk) → review+
Target Milestone: --- → Thunderbird 70.0
Attachment #9086832 - Flags: approval-comm-beta+

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/316f791941fd
do not assign a non-existent array elements in nsMsgSend::HackAttachments() and nsAbView::SwapFirstNameLastName(). r=jorgk

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: