Last Comment Bug 818250 - Possible crash with null m_identity in nsMsgCompose::ConvertAndLoadComposeWindow
: Possible crash with null m_identity in nsMsgCompose::ConvertAndLoadComposeWindow
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Composition (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Thunderbird 20.0
Assigned To: :aceman
:
:
Mentors:
Depends on:
Blocks:
  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:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


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

Description 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 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 :aceman 2012-12-05 05:31:04 PST
rkent, did you call nsMsgCompose::Initialize first?
Comment 3 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 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.
Comment 4 :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 Kent James (:rkent) 2012-12-05 15:27:13 PST
I would just error out.
Comment 6 :aceman 2012-12-06 11:53:14 PST
Created attachment 689324 [details] [diff] [review]
patch
Comment 7 Ryan VanderMeulen [:RyanVM] 2012-12-10 05:16:13 PST
https://hg.mozilla.org/comm-central/rev/bb60be2d8d3f

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