1181920 - XUL runs script under AttributeChanged notifications, when it's explicitly not really OK

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

Stack:

#1  0x0000000103081aec in mozilla::dom::AutoEntryScript::AutoEntryScript (this=0x7fff5fbfa9f8, aGlobalObject=0x12ad56340, aReason=0x107f51ae7 "XPCWrappedJS method call", aIsMainThread=true, aCx=0x0) at ScriptSettings.cpp:549
#2  0x0000000102696ce0 in nsXPCWrappedJSClass::CallMethod (this=0x12b49b560, wrapper=0x12adde980, methodIndex=4, info_=0x115d9ae88, nativeParams=0x7fff5fbfac40) at XPCWrappedJSClass.cpp:967
#3  0x0000000102696be5 in nsXPCWrappedJS::CallMethod (this=0x12adde980, methodIndex=4, info=0x115d9ae88, params=0x7fff5fbfac40) at XPCWrappedJS.cpp:525
#4  0x0000000101743583 in PrepareAndDispatch (self=0x12a2d5900, methodIndex=4, args=0x7fff5fbfad60, gpregs=0x7fff5fbface0, fpregs=0x7fff5fbfad10) at xptcstubs_x86_64_darwin.cpp:122
#5  0x0000000101741f8b in SharedStub () at nsDebug.h:81
#6  0x0000000105047752 in mozilla::dom::XULDocument::Persist (this=0x1285d6000, aElement=0x1285905e0, aNameSpaceID=0, aAttribute=0x11914a6d0) at XULDocument.cpp:1316
#7  0x0000000105046ce8 in mozilla::dom::XULDocument::AttributeChanged (this=0x1285d6000, aDocument=0x1285d6000, aElement=0x1285905e0, aNameSpaceID=0, aAttribute=0x11914a6d0, aModType=2) at XULDocument.cpp:1015
#8  0x000000010504798d in non-virtual thunk to mozilla::dom::XULDocument::AttributeChanged(nsIDocument*, mozilla::dom::Element*, int, nsIAtom*, int) (this=0x1285d6388, aDocument=0x1285d6000, aElement=0x1285905e0, aNameSpaceID=0, aAttribute=0x11914a6d0, aModType=2) at XULDocument.cpp:1019
#9  0x000000010321b9a5 in nsNodeUtils::AttributeChanged (aElement=0x1285905e0, aNameSpaceID=0, aAttribute=0x11914a6d0, aModType=2) at nsNodeUtils.cpp:141
#10 0x0000000103008df5 in mozilla::dom::Element::SetAttrAndNotify (this=0x1285905e0, aNamespaceID=0, aName=0x11914a6d0, aPrefix=0x0, aOldValue=@0x7fff5fbfb988, aParsedValue=@0x7fff5fbfb958, aModType=2 '\002', aFireMutation=false, aNotify=true, aCallAfterSetAttr=true) at Element.cpp:2322
#11 0x00000001030085fa in mozilla::dom::Element::SetAttr (this=0x1285905e0, aNamespaceID=0, aName=0x11914a6d0, aPrefix=0x0, aValue=@0x7fff5fbfbb08, aNotify=true) at Element.cpp:2194

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Persist XUL attributes off a scriptrunner

Comment 2

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/091a6a24c8d1

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/091a6a24c8d1

This probably should have had a security rating and/or sec-approval (if needed) before landing. Can we start with a rating at least?

Comment 4

[Boris Zbarsky [:bzbarsky]]

I would call this sec-other.  A malicious or incompetent extension could, with some effort, cause a security problem here, but...

The main reason this is closed, from my point of view, is to avoid drawing attention to the stack in comment 0, which might lead someone investigating it to find other cases where it _is_ a problem.  I haven't had a chance to investigate those yet, because I have yet to get a browser to start with the assertion involved.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Do we need to consider any backports?

Comment 6

[Boris Zbarsky [:bzbarsky]]

I don't think we should worry about backporting this.