1444369 - Crash in NS_strlen via nsTextFormatter::cvt_S

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Wayne Mery (:wsmwk)]

#12 crash for 59.0b2, ~#17 for 59.0b1.  crashs addresses are always 0x2f or 0x2e

Oldest crash I find is bp-7b389fe5-6807-48ba-b7d6-aabd40180208	2018-02-08 09:12:59 58.0b1 buildid 20171124151037


This bug was filed from the Socorro interface and is
report bp-7b389fe5-6807-48ba-b7d6-aabd40180208.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll NS_strlen xpcom/base/nsCRTGlue.cpp:83
1 xul.dll nsTextFormatter::cvt_S xpcom/string/nsTextFormatter.cpp:437
2 xul.dll nsTextFormatter::dosprintf xpcom/string/nsTextFormatter.cpp:822
3 xul.dll nsTextFormatter::vssprintf xpcom/string/nsTextFormatter.cpp:866
4 xul.dll nsTextFormatter::ssprintf<char16_t const*, char16_t const*, char16_t const*, char16_t const*, char16_t const*, char16_t const*, char16_t const*, char16_t const*, char16_t const*, char16_t const*> xpcom/string/nsTextFormatter.h:66
5 xul.dll nsStringBundle::FormatString intl/strres/nsStringBundle.cpp:304
6 xul.dll nsStringBundle::FormatStringFromName intl/strres/nsStringBundle.cpp:207
7 xul.dll nsImapMailFolder::Rename C:/builds/moz2_slave/tb-rel-c-beta-w32_bld-00000000/build/mailnews/imap/src/nsImapMailFolder.cpp:1601
8 xul.dll NS_InvokeByIndex xpcom/reflect/xptcall/md/win32/xptcinvoke_asm_x86_msvc.asm:54
9 xul.dll XPCWrappedNative::CallMethod js/xpconnect/src/XPCWrappedNative.cpp:1282

=============================================================

various comments all with similar theme
bp-9dbbe45c-010b-4332-bfe3-edb180180223	Changed Archive label
bp-f3924818-20ac-49ac-ba26-0ac120180223	I added a 2nd archive subfolder & it crashed	
bp-a4d6dc6b-672f-423e-87f2-b05a10180210	I changed folder from Financial to Financial / Bills and it crashed. I assume the "/" is not accepted as a character.
bp-18abd1bf-5e19-4bab-9859-03b0d0180309 Rename "2017" folder to "2017.9"

Comment 1

[Jorg K (CEST = GMT+2)]

Reading the code in nsImapMailFolder::Rename():
https://dxr.mozilla.org/comm-central/rev/1d4b272ca3135eceba64b451ee4b5738f74231a7/mailnews/imap/src/nsImapMailFolder.cpp#1581
the crash is caused when trying to rename a folder to a name that contains the hierarchy delimiter, typically ".".

I tried to rename a folder to a name with a dot and crashed straight away. Patch coming.

Comment 2

[Jorg K (CEST = GMT+2)]

Actually, I tried 
        // const char16_t delimiter[2] = { (char16_t)m_hierarchyDelimiter, '\0' };
        const char16_t delimiter[2] = { 'H', '\0' };
        const char16_t *formatStrings[] = { delimiter };


instead of
        const char16_t *formatStrings[] =
        {
          (const char16_t*)(intptr_t)m_hierarchyDelimiter
        };

and it still crashes.

Then I looked at the string and saw:
The %c character is reserved on this imap server. Please choose another name.

This %c is the only on in the entire Mozilla code base. Is %c no longer supported?

Changing this to %s obviously should work.

Comment 3

[Jorg K (CEST = GMT+2)]

Attachment: 1444369-no-more-percent-c.patch

Surprisingly %s in the format string still crashes, but %S works.

Comment 4

[Masatoshi Kimura [:emk]]

The char16_t* flavor of nsStringBundle::FormatString[1] happened to work with %c characters when sizeof(int)==sizeof(char16_t*) (a.k.a. 32-bit systems.)

>          (const char16_t*)(intptr_t)m_hierarchyDelimiter

This horrible hack will not work on 64-bit systems. I support changing the format specifier to "%S"

[1] https://dxr.mozilla.org/mozilla-central/source/intl/strres/nsStringBundle.cpp#304-314

Comment 5

[Jorg K (CEST = GMT+2)]

Comment on attachment 8957542 [details] [diff] [review]
1444369-no-more-percent-c.patch

Thanks for the comment. If you support it, please review it. Since this has a string change, it should land very soon.

Comment 6

[Jorg K (CEST = GMT+2)]

Comment on attachment 8957542 [details] [diff] [review]
1444369-no-more-percent-c.patch

Thanks!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/454ad0bb7a7e
fix crash renaming IMAP folder by changing %c to %S in format string. r=emk