Last Comment Bug 818250 - Possible crash with null m_identity in nsMsgCompose::ConvertAndLoadComposeWindow
: Possible crash with null m_identity in nsMsgCompose::ConvertAndLoadComposeWindow
Product: MailNews Core
Classification: Components
Component: Composition (show other bugs)
: Trunk
: All All
-- normal (vote)
: Thunderbird 20.0
Assigned To: :aceman
Depends on:
  Show dependency treegraph
Reported: 2012-12-04 13:25 PST by Kent James (:rkent)
Modified: 2012-12-10 05:16 PST (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

patch (1.14 KB, patch)
2012-12-06 11:53 PST, :aceman
neil: review+
Details | Diff | Splinter Review

Description User image Kent James (:rkent) 2012-12-04 13:25:34 PST
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.
Comment 1 User image Wayne Mery (:wsmwk, NI for questions) 2012-12-04 15:57:11 PST
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)
Comment 2 User image :aceman 2012-12-05 05:31:04 PST
rkent, did you call nsMsgCompose::Initialize first?
Comment 3 User image Kent James (:rkent) 2012-12-05 13:19:29 PST
(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 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.
Comment 4 User image :aceman 2012-12-05 13:41:53 PST
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.
Comment 5 User image Kent James (:rkent) 2012-12-05 15:27:13 PST
I would just error out.
Comment 6 User image :aceman 2012-12-06 11:53:14 PST
Created attachment 689324 [details] [diff] [review]
Comment 7 User image Ryan VanderMeulen [:RyanVM] 2012-12-10 05:16:13 PST

Note You need to log in before you can comment on or make changes to this bug.