884109 - Remove use of GetDocumentFromCaller in image and option code

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Bobby Holley (:bholley)]

Currently it calls into XPConnect, which uses the most recent XPCCallContext. But for consumers on new bindings, this answer is no longer correct.

In particular, I'm worried about NS_NewHTMLOptionElement and NS_NewHTMLImageElement, both of which are on new bindings (I believe) and use GetDocumentFromCaller to determine NodeInfo. I'm not sure what kind of nastiness can be done with an incorrect NodeInfo,  but I'm CCing moz_bug_r_a4 in case it proves fruitful.

I'm rewriting this function in one of my patches in bug 883450.

Comment 1

[Boris Zbarsky [:bzbarsky]]

NS_NewHTMLOptionElement and NS_NewHTMLImageElement got called with null aNodeInfo from CreateHTMLOptionElement and CreateHTMLImgElement.  These can only be invoked when constructing them via contractid or classid.

So first of all, in that situation we will in fact be coming from XPConnect and have an XPCCallContext.

Second, this used to be done by the |new Option| and |new Image| stuff, but now those are WebIDL constructors.  So I think we should just rip out the contract/classids for option and image, rip out the Create methods in nsLayoutModule.cpp, and rip out the special-casing in the NS_New methods: make them assume aNodeInfo is not null.  This is all dead code as far as the web is concerned.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Fully support removing support for contractid/classid for Img and Option.

Comment 3

[Boris Zbarsky [:bzbarsky]]

OK, let's morph this to cover that removal and this is not a security issue.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Remove GetDocumentFromCaller from NS_NewHTMLImage/OptionElement and remove the XPCOM-y bits for creating those, since WebIDL bindings don't use those.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Could you still check if some addon uses those contractids

Comment 6

[Boris Zbarsky [:bzbarsky]]

I did before writing the patch: no hits in the addons mxr.  Can't speak to binary addons...

Comment 7

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/aa768b37b1c8

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/aa768b37b1c8