The default bug view has changed. See this FAQ.

Possible crash with null m_identity in nsMsgCompose::ConvertAndLoadComposeWindow

RESOLVED FIXED in Thunderbird 20.0

Status

MailNews Core
Composition
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: rkent, Assigned: aceman)

Tracking

Trunk
Thunderbird 20.0

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

1.14 KB, patch
neil@parkwaycc.co.uk
: review+
Details | Diff | Splinter Review
(Reporter)

Description

4 years ago
In my ExQuilla binary extension, I'm managed to setup a situation where nsMsgCompose::ConvertAndLoadComposeWindow is being called with a null m_identity, which causes a crash. The core code should protect against this.
FWIW, there are no crashes in current versions with this signature. 

but 3.x releases did, as in bp-b94699c0-2945-4dfd-9d33-6484c2121124 with signature nsCOMPtr_base::~nsCOMPtr_base() | nsMsgCompose::ConvertAndLoadComposeWindow(nsString&, nsString&, nsString&, int, int)
(Assignee)

Comment 2

4 years ago
rkent, did you call nsMsgCompose::Initialize first?
(Reporter)

Comment 3

4 years ago
(In reply to :aceman from comment #2)
> rkent, did you call nsMsgCompose::Initialize first?

That would be an ExQuilla bug, and the underlying code is being revised. As a philosophy though, code that is easy to cause crashes in (particularly if the code is scriptable xpcom) should have checks added, as well as fixing the underlying cause.

This particular case is somewhat weaker since the idl entry for this call is marked [noscript]. But the fix is really easy. For my own purposes, since I do a C++ wrap of the compose code in my extension, I went ahead and added the fix in my own code.

See https://bitbucket.org/rkentjames/skinkglue/commits/8275ace4e927e50dabb81b4b6cf4e988 for what I ended up doing. For the fix for bug 615856 I added a patch for the core code as well, but I have not done one for this one since it is really rare.
(Assignee)

Comment 4

4 years ago
Should we error out in case of no m_identity or can we initialize it in some way? But we probably don't have the needed Params at hand.
(Reporter)

Comment 5

4 years ago
I would just error out.
(Assignee)

Comment 6

4 years ago
Created attachment 689324 [details] [diff] [review]
patch
Assignee: nobody → acelists
Status: NEW → ASSIGNED
Attachment #689324 - Flags: review?(neil)

Updated

4 years ago
Attachment #689324 - Flags: review?(neil) → review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
OS: Windows 7 → All
Hardware: x86_64 → All
Version: unspecified → Trunk
https://hg.mozilla.org/comm-central/rev/bb60be2d8d3f
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 20.0
You need to log in before you can comment on or make changes to this bug.