Closed Bug 1181920 Opened 5 years ago Closed 5 years ago

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

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox40 --- wontfix
firefox41 --- wontfix
firefox42 --- fixed
firefox-esr31 --- wontfix
firefox-esr38 --- wontfix
b2g-master --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Details

(Keywords: sec-other, Whiteboard: [post-critsmash-triage][adv-main42-])

Attachments

(1 file)

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
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #8631403 - Flags: review?(peterv) → review?(khuey)
Attachment #8631403 - Flags: review?(khuey) → review+
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?
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: needinfo?(bzbarsky)
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
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.
Flags: needinfo?(bzbarsky)
Do we need to consider any backports?
I don't think we should worry about backporting this.
Group: core-security → core-security-release
Whiteboard: [post-critsmash-triage]
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main42-]
Group: core-security-release
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.