Last Comment Bug 519901 - delete node.prop; gives "Security Manager vetoed action" (XPCNativeWrapper)
: delete node.prop; gives "Security Manager vetoed action" (XPCNativeWrapper)
Status: RESOLVED FIXED
[doc-waiting-1.9.3]
: dev-doc-complete
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: Trunk
: x86 Windows XP
: -- normal (vote)
: mozilla1.9.3a1
Assigned To: Blake Kaplan (:mrbkap)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-09-30 21:12 PDT by John J. Barton
Modified: 2010-08-19 06:52 PDT (History)
6 users (show)
mrbkap: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Proposed fix (941 bytes, patch)
2009-10-12 18:02 PDT, Blake Kaplan (:mrbkap)
bzbarsky: review+
Details | Diff | Splinter Review

Description John J. Barton 2009-09-30 21:12:15 PDT
On 9/30/09 12:55 AM, John J. Barton wrote:
> I hit a problem, I get an error message "Security Manager vetoed action"
> with the line number point to:
> delete node.__walkingAnonymousChildren; // unmark looping
> Here node is in an nsIDOMDocumentXBL document loaded in a content
> browser.

I assume |node| is just something that looks like a DOM node to you (as in, you didn't do wrappedJSObject on it or anything?

If so, looks like the issue is that XPC_NW_DelProperty will throw NS_ERROR_XPC_SECURITY_MANAGER_VETO, in general, when touching content from chrome.  Not sure why that code is as it is; this was part of the initial XPCNativeWrapper landing.  Looks wrong to me; worth filing a bug on and ccing ":jst" and "brendan@moz" and probably "mrbkap" and ":bz". Minimal-ish testcase:

javascript: var foo = new XPCNativeWrapper(document.body); foo.bar = 'x'; delete foo.bar;

Of course I'm not quite sure why you need the __walkingAnonymousChildren thing at all... Why do you need it?

-Boris
Comment 1 John J. Barton 2009-09-30 21:15:18 PDT
This code was probably working when the js was running in extensions space on extension nodes, but fails when the js is in extension space but the node is content.

Blocks nothing, I wrote different code.
Comment 2 Boris Zbarsky [:bz] 2009-09-30 21:38:28 PDT
I was talking about this with Blake, and we see no reason to forbid deletes on an XPCNativeWrapper from deleting own properties.  Brendan, jst, can you think of any such reasons?
Comment 3 Blake Kaplan (:mrbkap) 2009-10-12 18:02:31 PDT
Created attachment 405965 [details] [diff] [review]
Proposed fix

Note that unlike the other wrappers, XPCNativeWrapper doesn't forward the delete to its underlying object. If you delete an IDL-declared property, we'll simply re-resolve it the next time you ask for it. Expandos get nuked until you recreate them.
Comment 4 Boris Zbarsky [:bz] 2009-10-12 18:26:55 PDT
Comment on attachment 405965 [details] [diff] [review]
Proposed fix

r=bzbarsky
Comment 5 Blake Kaplan (:mrbkap) 2009-10-13 13:50:37 PDT
http://hg.mozilla.org/mozilla-central/rev/fae00beb30ff
Comment 6 Blake Kaplan (:mrbkap) 2009-10-13 14:21:36 PDT
Mochitests: http://hg.mozilla.org/mozilla-central/rev/2c61dc4844fe
Comment 7 Nickolay_Ponomarev 2009-10-19 03:28:27 PDT
So is this a fix for this limitation:
https://developer.mozilla.org/en/XPCNativeWrapper#Limitations_of_XPCNativeWrapper
"16. Using the delete operator on "expando" properties of an XPCNativeWrapper throws a security exception."
?
Comment 8 Boris Zbarsky [:bz] 2009-10-19 05:35:59 PDT
Yes, it is.

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