1907691 - Crash in [@ strlen | nsCharTraits<T>::length] via nsImapServerResponseParser::mailbox and nsImapServerResponseParser::ParseIMAPServerResponse(

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Wayne Mery (:wsmwk)]

Reporter of this crash indicates importing outlook data. states "Thunderbird updated, and now refuses to start, so I now have no access to my e-mail. "

Crash report: https://crash-stats.mozilla.org/report/index/cdaef368-8b27-470e-a5a9-4f2af0240711

Reason: EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames:

0  ucrtbase.dll  strlen
1  xul.dll  nsCharTraits<char>::length(char const*)  xpcom/string/nsCharTraits.h:423
1  xul.dll  nsTDependentString<char>::nsTDependentString(char const*)  xpcom/string/nsTDependentString.h:76
1  xul.dll  nsImapServerResponseParser::mailbox(nsImapMailboxSpec*)  mailnews/imap/src/nsImapServerResponseParser.cpp:788
2  xul.dll  nsImapServerResponseParser::mailbox_list(bool)  mailnews/imap/src/nsImapServerResponseParser.cpp:772
3  xul.dll  nsImapServerResponseParser::response_data()  mailnews/imap/src/nsImapServerResponseParser.cpp:447
4  xul.dll  nsImapServerResponseParser::ParseIMAPServerResponse(char const*, bool, char*)  mailnews/imap/src/nsImapServerResponseParser.cpp:189
5  xul.dll  nsImapProtocol::List(char const*, bool, bool)  mailnews/imap/src/nsImapProtocol.cpp:7676
6  xul.dll  nsImapProtocol::DiscoverMailboxList()  mailnews/imap/src/nsImapProtocol.cpp:7267
7  xul.dll  nsImapProtocol::ProcessAuthenticatedStateURL()  mailnews/imap/src/nsImapProtocol.cpp:7893

Comment 1

[Wayne Mery (:wsmwk)]

This isn't a new crash signature, but the stack might be new.

Comment 2

[Magnus Melin [:mkmelin]]

https://searchfox.org/comm-central/rev/03e5119a09c3feee0c67484fe23705f9723e6e73/mailnews/imap/src/nsImapServerResponseParser.cpp#788
CreateAstring() at https://searchfox.org/comm-central/rev/03e5119a09c3feee0c67484fe23705f9723e6e73/mailnews/imap/src/nsImapGenericParser.cpp#172 must be returning null
We should check null before using it for nsDependentCString - which will crash when sent null as parameter.

Comment 3

[Wayne Mery (:wsmwk)]

A few users' crash comments state they only started crashing with version 128.
bp-63601022-3113-461e-9dff-6400a0240617 is using davmail and beta builds.
Crashes are often at startup

Comment 4

[Magnus Melin [:mkmelin]]

Attachment: Bug 1907691 - Fix Crash in [@ strlen | nsCharTraits<T>::length] via nsImapServerResponseParser::mailbox and nsImapServerResponseParser::ParseIMAPServerResponse. r=tobyp

Comment 5

[Wayne Mery (:wsmwk)]

How would you feel about taking this to today's beta? Or, does it need a couple days on daily?

Comment 6

[Wayne Mery (:wsmwk)]

Any thoughts on a possible workaround, given that this often happens on startup?

Comment 7

[Magnus Melin [:mkmelin]]

It shouldn't be a problem but I would give it two days or so on daily.
I don't know exactly what would cause it.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/5dbcfc60b279
Fix Crash in [@ strlen | nsCharTraits<T>::length] via nsImapServerResponseParser::mailbox and nsImapServerResponseParser::ParseIMAPServerResponse. r=tobyp

Comment 9

[Magnus Melin [:mkmelin]]

Comment on attachment 9424619 [details]
Bug 1907691 - Fix Crash in [@ strlen | nsCharTraits<T>::length] via nsImapServerResponseParser::mailbox and nsImapServerResponseParser::ParseIMAPServerResponse. r=tobyp

[Approval Request Comment]
User impact if declined: may crash
Testing completed (on c-c, etc.): c-c
Risk to taking this patch (and alternatives if risky): should be safe

Comment 10

[Corey Bryant]

Comment on attachment 9424619 [details]
Bug 1907691 - Fix Crash in [@ strlen | nsCharTraits<T>::length] via nsImapServerResponseParser::mailbox and nsImapServerResponseParser::ParseIMAPServerResponse. r=tobyp

[Triage Comment]
Approved for beta

Comment 11

[Daniel Darnell [:dandarnell]]

Thunderbird 131.0b5:
https://hg.mozilla.org/releases/comm-beta/rev/6c21e3299b4e

Comment 12

[Corey Bryant]

Comment on attachment 9424619 [details]
Bug 1907691 - Fix Crash in [@ strlen | nsCharTraits<T>::length] via nsImapServerResponseParser::mailbox and nsImapServerResponseParser::ParseIMAPServerResponse. r=tobyp

[Triage Comment]
Approved for esr128

Comment 13

[Daniel Darnell [:dandarnell]]

Thunderbird 128.3.0esr:
https://hg.mozilla.org/releases/comm-esr128/rev/4f004e83ed9f

Comment 14

[Kai Engert [:KaiE:]]

I looks like this crash bug got introduced by bug 1907691, which uses nsDependentCString with a null string parameter.

The patch for bug 1853365 needs to get re-reviewed, to check for other places where nsDependentCString might get potentially used with a null pointer.

Comment 15

[Magnus Melin [:mkmelin]]

Please back out https://hg.mozilla.org/releases/comm-esr128/rev/4f004e83ed9f from 128. We'll handle the rest in bug 1921739.

Comment 16

[Kai Engert [:KaiE:]]

https://hg.mozilla.org/releases/comm-esr128/rev/36457f5e26f4499fa630a7d1909613c5ec148383

Comment 17

[Rob Lemley [:rjl]]

Comment on attachment 9424619 [details]
Bug 1907691 - Fix Crash in [@ strlen | nsCharTraits<T>::length] via nsImapServerResponseParser::mailbox and nsImapServerResponseParser::ParseIMAPServerResponse. r=tobyp

Removing approval flag per comments 15-16.

Comment 18

[Rob Lemley [:rjl]]

Thunderbird 128.3.2esr:
https://hg.mozilla.org/releases/comm-esr128/rev/89d4cf9ebada

Comment 19

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

Hello,

Are there any specific STR that QA could perform in order to reproduce the faulty behaviour(crash) on an affected build and properly verify and confirm that it is fixed?