622092 - crash [@ memcpy | mime_insert_all_headers], [@ mime_insert_all_headers] (Mac)

Status: RESOLVED WORKSFORME

(MailNews Core :: MIME, defect)

Description

[Wayne Mery (:wsmwk)]

crash [@ memcpy | mime_insert_all_headers], [@ mime_insert_all_headers] (Mac)

Most common comment is "forwarding message".
windows memcpy | mime_insert_all_headers and Mac mime_insert_all_headers
no crashes found for trunk users, but branch crash numbers are low enough that it this isn't a surprise.

bp-be19b445-cde9-4157-805e-2d4f42101110 (pierre) crash comments in french
EXCEPTION_ACCESS_VIOLATION_READ
0x8900000
0	mozcrt19.dll	memcpy	memcpy.asm:188
1	thunderbird.exe	mime_insert_all_headers	mailnews/mime/src/mimedrft.cpp:817
2	thunderbird.exe	mime_insert_forwarded_message_headers	mailnews/mime/src/mimedrft.cpp:1206
3	thunderbird.exe	mime_parse_stream_complete	mailnews/mime/src/mimedrft.cpp:1548
4	thunderbird.exe	nsStreamConverter::OnStopRequest	mailnews/mime/src/nsStreamConverter.cpp:1090
5	thunderbird.exe	nsMsgProtocol::OnStopRequest	mailnews/base/util/nsMsgProtocol.cpp:401
6	thunderbird.exe	nsMailboxProtocol::OnStopRequest	mailnews/local/src/nsMailboxProtocol.cpp:381
7	thunderbird.exe	nsInputStreamPump::OnStateStop	netwerk/base/src/nsInputStreamPump.cpp:578
8	thunderbird.exe	nsInputStreamPump::OnInputStreamReady	netwerk/base/src/nsInputStreamPump.cpp:403
9	xpcom_core.dll	nsOutputStreamReadyEvent::Run	xpcom/io/nsStreamUtils.cpp:112
10	xpcom_core.dll	nsThread::ProcessNextEvent	xpcom/threads/nsThread.cpp:527 


bp-dd299310-1daf-48f0-9dd7-50bc82101224 Mac 
EXC_BAD_ACCESS / KERN_INVALID_ADDRESS
0x25cffff0
0		@0xffff0f2c	
1	thunderbird-bin	mime_insert_all_headers	mailnews/mime/src/mimedrft.cpp:817
2	thunderbird-bin	mime_parse_stream_complete	mailnews/mime/src/mimedrft.cpp:1206
3	thunderbird-bin	nsStreamConverter::OnStopRequest	mailnews/mime/src/nsStreamConverter.cpp:1090
4	thunderbird-bin	nsMsgProtocol::OnStopRequest	mailnews/base/util/nsMsgProtocol.cpp:401
5	thunderbird-bin	nsMailboxProtocol::OnStopRequest	mailnews/local/src/nsMailboxProtocol.cpp:381
6	thunderbird-bin	nsInputStreamPump::OnStateStop	netwerk/base/src/nsInputStreamPump.cpp:578
7	thunderbird-bin	nsInputStreamPump::OnInputStreamReady	netwerk/base/src/nsInputStreamPump.cpp:403
8	libxpcom_core.dylib	nsInputStreamReadyEvent::Run	xpcom/io/nsStreamUtils.cpp:112
9	libxpcom_core.dylib	nsThread::ProcessNextEvent	xpcom/threads/nsThread.cpp:527
10	libxpcom_core.dylib	NS_ProcessPendingEvents_P	nsThreadUtils.cpp:200

Comment 1

[Makoto Kato [:m_kato]]

When the end of buffer is space (0x20), this crash can occurs.

Comment 2

[Wayne Mery (:wsmwk)]

m_kato, can you suggest a patch?

Comment 3

[Makoto Kato [:m_kato]]

Attachment: Fix

Comment 4

[Joshua Cranmer [:jcranmer]]

Comment on attachment 8357590 [details] [diff] [review]
Fix

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

Please include a test case that causes a crash without this patch.

Comment 5

[:aceman]

Comment on attachment 8357590 [details] [diff] [review]
Fix

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

Do we really need a test case here?

::: mailnews/mime/src/mimedrft.cpp
@@ -715,5 @@
>      }
>  
> -    /* Skip over whitespace after colon. */
> -    while (contents <= end && IS_SPACE(*contents))
> -    contents++;

So this allowed contents to be 1 past 'end' if there were just spaces at the end? Is that the bug and cause of the crash when we accessed contents later?

Comment 6

[Wayne Mery (:wsmwk)]

(In reply to :aceman from comment #5)
> Do we really need a test case here?

why wouldn't we want one?

Comment 7

[:aceman]

Of course it would be fine to have one, but do we want to wait another 4 years for it? Isn't the bug and fix known? I don't say it is as I am not familiar with the code. I'm just asking.

Comment 8

[Wayne Mery (:wsmwk)]

(In reply to :aceman from comment #7)
> Of course it would be fine to have one, but do we want to wait another 4
> years for it? Isn't the bug and fix known? I don't say it is as I am not
> familiar with the code. I'm just asking.

4 years for a fix doesn't mean it'd take 4 years to make a test :)

But it's up to Joshua

Comment 9

[Wayne Mery (:wsmwk)]

m_kato, can you construct a testcase?

Comment 10

[Makoto Kato [:m_kato]]

Joshua, as long as I know, you have a plan that you replace old netscape's MIME code with JS's code.  When?  next major release?

If next major release replaces with js's code, this bug may be unnecessary.

Comment 11

[Joshua Cranmer [:jcranmer]]

(In reply to Makoto Kato (:m_kato) from comment #10)
> Joshua, as long as I know, you have a plan that you replace old netscape's
> MIME code with JS's code.  When?  next major release?

Nowhere near that soon, unfortunately.

Comment 12

[Wayne Mery (:wsmwk)]

question for m_kato?

(In reply to :aceman from comment #5)
> Comment on attachment 8357590 [details] [diff] [review]
> Fix
> 
> Review of attachment 8357590 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Do we really need a test case here?
> 
> ::: mailnews/mime/src/mimedrft.cpp
> @@ -715,5 @@
> >      }
> >  
> > -    /* Skip over whitespace after colon. */
> > -    while (contents <= end && IS_SPACE(*contents))
> > -    contents++;
> 
> So this allowed contents to be 1 past 'end' if there were just spaces at the
> end? Is that the bug and cause of the crash when we accessed contents later?

Comment 13

[Makoto Kato [:m_kato]]

Wayne, I cannot find same signage now.  Does this still occur?

Comment 14

[Wayne Mery (:wsmwk)]

Indeed, no crashes after 24.8.1, for example bp-04e292eb-955f-42bd-b21e-2495b2151208.
Either the signature changed or joshua/someone fixed something

what's left in mime-world are bug 564701, bug 469087, bug 547621, bug 844647.