Closed Bug 289940 Opened 20 years ago Closed 20 years ago

Chrome code needs to be protected from untrusted events.

Categories

(Core :: DOM: Events, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED

People

(Reporter: jst, Assigned: jst)

References

Details

(Keywords: fixed-aviary1.0.5, fixed1.7.9, Whiteboard: [sg:fix] need branch landing , needed for b2 cuz it breaks things)

Attachments

(5 files, 2 obsolete files)

Right now our chrome code generally excpects its event handlers to be called onyl from the code that normally fires the events in question, but lots of event handlers listen for events that can be synthesized by untrusted content, and we're thus vulnerable to security problems, or at least unexpected code execution. We have a solution for this problem, and that is to check if the event in question is trusted or not, but way too often this check is omitted and it's by now unlikely to expect this problem to be fixable by adding those checks, and not forgetting those checks in new code etc. The safer fix for this problem is to simply stop delivering untrusted events to chrome code. This way, if people forget something related to this, their code won't work rather than having security problems creep into the code. The potential problem with this approach is that existing code that does rely on untrusted events in chrome code is broken by this change. But I think that's inevitable, and I'm hoping the breakage won't be too broad. Because of this, we do need an alternate apporach that the code that does indeed need to see untrusted events from chrome code can use. That will be done by an optional additional argument to the addEventListener() method that tells whether the listener wants untrusted events or not. By default content code will get them, chrome code won't. The big part of this change is to mark all our internal events as trusted, where applicable. This will be done by changing the constructors of the various ns*Event classes such that the C++ compiler will require you to specify if an event is trusted or not. Patch coming up (tested only on Win32 and Gtk2 builds for now). (marking security sensitive to keep the visibility of this problem down for now)
/mozilla/widget/src/gtk/nsWindow.cpp:3179: no matching function for call to `nsMouseEvent::nsMouseEvent(int, int, nsWindow*&)' /mozilla/widget/src/gtk/nsWindow.cpp:3318: no matching function for call to `nsMouseEvent::nsMouseEvent(int, int, nsWindow*&)' With those fixed, widget/ builds. I'll do a build of the whole tree tonight and see whether I can give it a spin.
Whiteboard: [sg:fix]
More build bustage, only if SVG is enabled: /mozilla/content/xml/document/src/nsXMLContentSink.cpp:1008: no matching function for call to `nsEvent::nsEvent(int)'
And more: /mozilla/content/svg/content/src/nsSVGElement.cpp:234: no matching function for call to `nsDerivedSafe<nsIEventListenerManager>:: AddScriptEventListener(nsIContent*, nsIAtom*&, const nsAString&, int)' /mozilla/content/svg/content/src/nsSVGElement.cpp:668: no matching function for call to `nsMutationEvent::nsMutationEvent(int, nsCOMPtr<nsIDOMEventTarget>&)' /mozilla/content/svg/content/src/nsSVGScriptElement.cpp:242: no matching function for call to `nsScriptErrorEvent::nsScriptErrorEvent(int)' /mozilla/content/svg/content/src/nsSVGScriptElement.cpp:283: no matching function for call to `nsEvent::nsEvent(int)'
With those 7 spots fixed, a GTK1 build compiles and runs. I tested some things (clicking on links, the link toolbar, that sort of thing) and they worked.
*** Bug 265729 has been marked as a duplicate of this bug. ***
This merges attachment 180396 [details] [diff] [review] to the trunk (changes to nsEventStateManager), makes the changes per comment 2 (untested, since built on Mac), comment 3, and comment 4, and makes some changes to nsMenuX.cpp and nsMenuBarX.cpp in widget/src/mac/ to get this to build Mac firefox and to nsMenuX.cpp in widget/src/cocoa/ for building Camino (my Camino build isn't done yet, but it's past widget; my Mac firefox build failed linking libgklayout.dylib for reasons related to cairo).
Attachment #180396 - Attachment is obsolete: true
Attachment #181474 - Flags: review?(bugmail)
I've got this up and running locally, still reviewing though...
I feel a bit uneasy about the iscallerchrome check in RegisterScriptEventListener, but it's probably ok. I need to look at it some more. In @@ -1620,32 +1632,44 @@ nsresult nsEventListenerManager::HandleE + if (!NS_IS_TRUSTED_EVENT(aEvent) || + ls->mFlags & NS_PRIV_EVENT_UNTRUSTED_PERMITTED) { + int a = 4; + } ... + } else { + int a = 5; } Debug code? In @@ -524,19 +524,19 @@ nsEventStateManager::PreHandleEvent(nsPr - nsEvent blurevent(NS_BLUR_CONTENT); + nsEvent blurevent(NS_IS_TRUSTED_EVENT(aEvent), NS_BLUR_CONTENT); Couldn't this always be trusted? Even if the focusevent was untrusted we're still really blurring the old element, right? Same in other places in this file. Hmm.. is this a problem in general, that 'fake' events can have real effects that chrome is interested in. Like if a javascript clickevent is dispatched on a link.
Links actually ignore all untrusted click events in current builds...
So are there no untrusted events we want to react to? How about clicks on sumbitbuttons? And can focus affect the UI? In @@ -1267,21 +1267,24 @@ nsEventStateManager::FireContextClick() + // XXX: Should this be a trusted event? + nsMouseEvent event(PR_TRUE, NS_CONTEXTMENU, + mCurrentTarget->GetWindow(), + nsMouseEvent::eReal); Why not?
Somewhat related to this bug, but should be a separate patch. Is there a reason we couldn't make sure that all eventclasses enforce that event.target is always a real node (rather then a js object).
> So are there no untrusted events we want to react to? I want to say "no", but I'm not sure that's true.... > How about clicks on sumbitbuttons? That's a tough one (since the page can just call submit() on the form, which does a similar but not identical thing). > And can focus affect the UI? Probably. :( ccing some UI folks for help.
(In reply to comment #12) > Somewhat related to this bug, but should be a separate patch. Is there a reason > we couldn't make sure that all eventclasses enforce that event.target is always > a real node (rather then a js object). Have a look at nsEventStateManager::DispatchNewEvent and its caller. I *think* this is already done.
(In reply to comment #13) > > So are there no untrusted events we want to react to? > > I want to say "no", but I'm not sure that's true.... I can't say I'm sure either, but I must say I'd be surprised if this wasn't the case. > > How about clicks on sumbitbuttons? > > That's a tough one (since the page can just call submit() on the form, which > does a similar but not identical thing). Not really, IMO. No matter how a submit happens, the onsubmit event should be trusted since it's an event that tells you that we're now submitting, no matter what triggerd the submit action (i.e. a synthetic click event on a submit button should fire a trusted onsubmit event, just like calling form.submit() should).
In @@ -1629,19 +1638,20 @@ nsHTMLInputElement::HandleDOMEvent(nsPre if (mForm) { - nsFormEvent event((mType == NS_FORM_INPUT_RESET) ? + // XXX: Should this always be trusted? + nsFormEvent event(PR_TRUE, (mType == NS_FORM_INPUT_RESET) ? NS_FORM_RESET : NS_FORM_SUBMIT); This is IMHO inconsistent. Content calling Reset() will give an untrusted reset/submit event, but content dispatching a DOMActivate will get a trusted one. I think I agree that these could be trusted. Same inconsistency in xulelement::click/docommand vs. html*element::click, but here I think a CallerIsChrome check is the right thing to do. In @@ -668,19 +665,20 @@ nsXULElement::AddScriptEventListener(nsI - return manager->AddScriptEventListener(target, aName, aValue, defer); + return manager->AddScriptEventListener(target, aName, aValue, defer, + /* XXX */ PR_TRUE); Shouldn't this have the SchemeIs("chrome") check? In @@ -373,19 +373,21 @@ nsXULCommandDispatcher::UpdateCommands(c + // XXX: Do we need to check if we're called from chrome? + nsEvent event(PR_TRUE, NS_XUL_COMMAND_UPDATE); It'd be good if someone that knew the command architecture better then me answered this question. You'll have to walk me through the classinfo magic. I get most of it, but not all. In @@ -3495,19 +3495,20 @@ nsresult nsPluginInstanceOwner::Dispatch - nsGUIEvent focusEvent(theEvent->message); // we only care about the message in ProcessEvent + // we only care about the message in ProcessEvent + nsGUIEvent focusEvent(PR_FALSE, theEvent->message, nsnull); Why false? Are you sure it's ok to remove the nullcheck in nsMenuFrame::Execute? Ok, it's getting too late here. Finishing the rest tomorrow.
(In reply to comment #13) >>And can focus affect the UI? >Probably. :( ccing some UI folks for help. I'm probably commenting at completely the wrong time of day, so all I can think of is that we currently update the status bar with a link's target as it gets focus (although but we don't clear it when it blurs...)
I wonder if we should always make focus events that actually affect focus trusted too? Just like submit and reset. In @@ -1507,19 +1508,19 @@ void nsGfxScrollFrameInner::CurPosAttrib - nsScrollbarEvent event(NS_SCROLL_EVENT); + nsScrollbarEvent event(PR_FALSE, NS_SCROLL_EVENT, nsnull); Why false? Shouldn't the code in nsButtonBoxFrame::MouseClicked default to untrusted event if there is no aEvent. Similar question in other MouseClicked functions. Dbaron/bz, does MouseClicked (with or without an aEvent argument) ever get called unless the user actually clicked on the frame using the mouse or through accessibility interfaces? Maybe we could use check for js on the stack and make it untrusted unless the top js is chrome?
Flags: blocking1.8b2+
Blocks: 289961
The added space in @@ -2638,20 +2638,20 @@ nsChildView::Idle() seems accidental Whitespace in @@ -1548,30 +1550,32 @@ PRBool nsMacEventHandler::HandleMouseDow is whacky You claim all eventtargets should implement nsIDOMNSEventTarget, but you're not adding that interface to any classes? Ok, that's it!
Whiteboard: [sg:fix] → [sg:fix] needed for b2 cuz it breaks things
(In reply to comment #19) > The added space in @@ -2638,20 +2638,20 @@ nsChildView::Idle() seems accidental Yeah, fixed. > Whitespace in @@ -1548,30 +1550,32 @@ PRBool nsMacEventHandler::HandleMouseDow > is whacky Yeah, whitespace in that whole file is whacky. Nice mixture of spaces and tabs. I won't mess with that now. > You claim all eventtargets should implement nsIDOMNSEventTarget, but you're not > adding that interface to any classes? Oops, yeah, that was the intention, but I never wrote that code. I've got it now tho... new patch coming up where all your comments have been addressed, and some more issues too.
Status: NEW → ASSIGNED
Reviewers, FYI there's a missing '!' in front of the isChromeElement argument in nsXULElement::AddScriptEventListener(), the code should be: return manager->AddScriptEventListener(target, aName, aValue, defer, !isChromeElement);
Comment on attachment 181923 [details] [diff] [review] Same thing, but fix CreateEvent() users too, and remove unused MouseClicked() methods in some layout code, and implement nsIDOMNSEventTarget::AddEventListener() on all targets. hoping we get review overnight and can get this approved on thursday if it looks good.
Attachment #181923 - Flags: approval1.8b2?
Comment on attachment 181923 [details] [diff] [review] Same thing, but fix CreateEvent() users too, and remove unused MouseClicked() methods in some layout code, and implement nsIDOMNSEventTarget::AddEventListener() on all targets. In @@ -343,43 +348,48 @@ nsHTMLButtonElement::HandleDOMEvent(nsPr Should the last event in this block always be trusted? Or should we promote the event to a trusted one sometime later on if it in fact causes a submission/reset (does it ever not?). For the various click functions, I think it might be a good idea to implement a nsContentUtils::IsCallerNativeOrChrome(). There might be embedders that want to call these functions. So is nsEventListenerManager::RegisterScriptEventListener the function that is called when you set myElem.onfoo = <jscode> ? Please add some documentation to the headerfile that explains that this function will do a iscallerchrome check. r=me with that
Attachment #181923 - Flags: review?(bugmail) → review+
Comment on attachment 181923 [details] [diff] [review] Same thing, but fix CreateEvent() users too, and remove unused MouseClicked() methods in some layout code, and implement nsIDOMNSEventTarget::AddEventListener() on all targets. >Index: content/events/public/nsIPrivateDOMEvent.h >=================================================================== >+class nsEvent; >+NS_NewDOMEvent(nsIDOMEvent** aInstancePtrResult, nsPresContext* aPresContext, class nsEvent *aEvent); Don't need the class keyword here. >Index: content/events/src/nsDOMUIEvent.cpp >=================================================================== >+ : nsDOMEvent(aPresContext, aEvent ? >+ NS_STATIC_CAST(nsEvent *, aEvent) : >+ NS_STATIC_CAST(nsEvent *, new nsUIEvent(PR_FALSE, 0, 0))) Are these casts needed? >Index: content/events/src/nsEventListenerManager.cpp >=================================================================== > nsListenerStruct* > nsEventListenerManager::FindJSEventListener(EventArrayType aType) > { > nsVoidArray *listeners = GetListenersByType(aType, nsnull, PR_FALSE); > if (listeners) { >- //Run through the listeners for this IID and see if a script listener is registered >+ // Run through the listeners for this type and see if a script >+ // listener is registered > nsListenerStruct *ls; >- for (int i=0; i<listeners->Count(); i++) { >+ for (int i=0; i < listeners->Count(); i++) { While you're here: s/int/PRInt32/ >@@ -1620,32 +1632,44 @@ nsresult nsEventListenerManager::HandleE > PRInt32 count = listeners->Count(); > nsVoidArray originalListeners(count); > originalListeners = *listeners; > > nsAutoPopupStatePusher popupStatePusher(nsDOMEvent::GetEventPopupControlState(aEvent)); > > for (int k = 0; !mListenersRemoved && listeners && k < count; ++k) { > nsListenerStruct* ls = NS_STATIC_CAST(nsListenerStruct*, originalListeners.FastElementAt(k)); > // Don't fire the listener if it's been removed >- if (listeners->IndexOf(ls) != -1 && ls->mFlags & aFlags && ls->mGroupFlags == currentGroup) { >+ >+ if (!NS_IS_TRUSTED_EVENT(aEvent) || >+ ls->mFlags & NS_PRIV_EVENT_UNTRUSTED_PERMITTED) { >+ int a = 4; >+ } Remove this. > // If it doesn't implement that, call the generic HandleEvent() > if (!hasInterface && (ls->mSubType == NS_EVENT_BITS_NONE || >- ls->mSubType & dispData->bits)) >+ ls->mSubType & dispData->bits)) { > HandleEventSubType(ls, *aDOMEvent, aCurrentTarget, > dispData ? dispData->bits : NS_EVENT_BITS_NONE, > aFlags); >+ } >+ } else { >+ int a = 5; Remove this. >Index: content/events/src/nsEventStateManager.cpp >=================================================================== >@@ -3985,28 +3987,31 @@ nsEventStateManager::SendFocusBlur(nsPre > nsCOMPtr<nsIViewManager> kungFuDeathGrip; > nsIPresShell *shell = doc->GetShellAt(0); > if (shell) { > kungFuDeathGrip = shell->GetViewManager(); > > nsCOMPtr<nsPresContext> oldPresContext = shell->GetPresContext(); > > //fire blur > nsEventStatus status = nsEventStatus_eIgnore; >- nsEvent event(NS_BLUR_CONTENT); >+ nsEvent event(nsContentUtils::IsCallerChrome(), NS_BLUR_CONTENT); > > EnsureDocument(presShell); > >- // Make sure we're not switching command dispatchers, if so, surpress the blurred one >+ // Make sure we're not switching command dispatchers, if so, >+ // surpress the blurred one > if(gLastFocusedDocument && mDocument) { > nsIFocusController *newFocusController = nsnull; > nsIFocusController *oldFocusController = nsnull; >- nsCOMPtr<nsPIDOMWindow> newWindow = do_QueryInterface(mDocument->GetScriptGlobalObject()); >- nsCOMPtr<nsPIDOMWindow> oldWindow = do_QueryInterface(gLastFocusedDocument->GetScriptGlobalObject()); >+ nsCOMPtr<nsPIDOMWindow> newWindow = >+ do_QueryInterface(mDocument->GetScriptGlobalObject()); >+ nsCOMPtr<nsPIDOMWindow> oldWindow = >+ do_QueryInterface(gLastFocusedDocument->GetScriptGlobalObject()); > if(newWindow) > newFocusController = newWindow->GetRootFocusController(); > if(oldWindow) > oldFocusController = oldWindow->GetRootFocusController(); > if(oldFocusController && oldFocusController != newFocusController) > oldFocusController->SetSuppressFocus(PR_TRUE, "SendFocusBlur Window Switch"); > } > > nsCOMPtr<nsIEventStateManager> esm; >@@ -4040,21 +4045,22 @@ nsEventStateManager::SendFocusBlur(nsPre > nsCOMPtr<nsIScriptGlobalObject> globalObject; > > if(gLastFocusedDocument) > globalObject = gLastFocusedDocument->GetScriptGlobalObject(); > > EnsureDocument(presShell); > > if (gLastFocusedDocument && (gLastFocusedDocument != mDocument) && globalObject) { > nsEventStatus status = nsEventStatus_eIgnore; >- nsEvent event(NS_BLUR_CONTENT); >+ nsEvent event(nsContentUtils::IsCallerChrome(), NS_BLUR_CONTENT); > >- // Make sure we're not switching command dispatchers, if so, surpress the blurred one >+ // Make sure we're not switching command dispatchers, if so, >+ // surpress the blurred one > if (mDocument) { > nsIFocusController *newFocusController = nsnull; > nsIFocusController *oldFocusController = nsnull; > nsCOMPtr<nsPIDOMWindow> newWindow = do_QueryInterface(mDocument->GetScriptGlobalObject()); > nsCOMPtr<nsPIDOMWindow> oldWindow = do_QueryInterface(gLastFocusedDocument->GetScriptGlobalObject()); > > if (newWindow) > newFocusController = newWindow->GetRootFocusController(); > oldFocusController = oldWindow->GetRootFocusController(); >@@ -4124,19 +4130,19 @@ nsEventStateManager::SendFocusBlur(nsPre > //to that element while the first focus is still ongoing. > PRBool clearFirstFocusEvent = PR_FALSE; > if (!mFirstFocusEvent) { > mFirstFocusEvent = aContent; > clearFirstFocusEvent = PR_TRUE; > } > > //fire focus > nsEventStatus status = nsEventStatus_eIgnore; >- nsEvent event(NS_FOCUS_CONTENT); >+ nsEvent event(nsContentUtils::IsCallerChrome(), NS_FOCUS_CONTENT); > > if (nsnull != mPresContext) { > nsCxPusher pusher(aContent); > aContent->HandleDOMEvent(mPresContext, &event, nsnull, NS_EVENT_FLAG_INIT, &status); > } > > nsAutoString tabIndex; > aContent->GetAttr(kNameSpaceID_None, nsHTMLAtoms::tabindex, tabIndex); > PRInt32 ec, val = tabIndex.ToInteger(&ec); >@@ -4145,19 +4151,19 @@ nsEventStateManager::SendFocusBlur(nsPre > } > > if (clearFirstFocusEvent) { > mFirstFocusEvent = nsnull; > } > } else if (!aContent) { > //fire focus on document even if the content isn't focusable (ie. text) > //see bugzilla bug 93521 > nsEventStatus status = nsEventStatus_eIgnore; >- nsEvent event(NS_FOCUS_CONTENT); >+ nsEvent event(nsContentUtils::IsCallerChrome(), NS_FOCUS_CONTENT); > > if (nsnull != mPresContext && mDocument) { > nsCxPusher pusher(mDocument); > mDocument->HandleDOMEvent(mPresContext, &event, nsnull, NS_EVENT_FLAG_INIT, &status); > } > } > > if (mBrowseWithCaret) > SetContentCaretVisible(presShell, aContent, PR_TRUE); I don't understand why we check for IsCallerChrome in these. >Index: dom/public/idl/events/nsIDOMNSEventTarget.idl >=================================================================== >+ * The nsIDOMNSEventTarget interface an extension to the standard ... interface *is* an ... >+ * lets callers controll whether or not they want to receive control >+ * they're truested trusted >Index: dom/src/base/nsDOMClassInfo.cpp >=================================================================== > nsresult > nsEventReceiverSH::RegisterCompileHandler(nsIXPConnectWrappedNative *wrapper, > JSContext *cx, JSObject *obj, > jsval id, PRBool compile, > PRBool remove, >- PRBool *did_compile) >+ PRBool *did_define) > if (!IsEventName(id)) { >+ if (id == sAddEventListener_id) { >+ JSString *str = JSVAL_TO_STRING(id); >+ JSFunction *fnc = >+ ::JS_DefineFunction(cx, obj, ::JS_GetStringBytes(str), >+ AddEventListenerHelper, 0, JSPROP_ENUMERATE); >+ >+ *did_define = PR_TRUE; >+ >+ return fnc ? NS_OK : NS_ERROR_UNEXPECTED; >+ >+ } >+ > return NS_OK; > } I'd move this into NewResolve Please file a followup bug on the XUL command stuff and the XForms stuff, so we can make sure we're doing the right thingthere.
Attachment #181923 - Flags: superreview?(peterv) → superreview+
silver says nsEvent.h declares nsEvent as a struct. please don't forward declare it incorrectly, that'll trigger a warning.
I wonder if these too should always be trusted: @@ -577,19 +577,19 @@ nsEventStateManager::PreHandleEvent(nsPr @@ -4124,19 +4130,19 @@ nsEventStateManager::SendFocusBlur(nsPre @@ -4145,19 +4151,19 @@ nsEventStateManager::SendFocusBlur(nsPre @@ -255,19 +255,19 @@ nsHTMLLabelElement::HandleDOMEvent(nsPre Not really sure about the last one...
Ugh, and these: @@ -524,19 +524,19 @@ nsEventStateManager::PreHandleEvent(nsPr @@ -675,19 +675,19 @@ nsEventStateManager::PreHandleEvent(nsPr @@ -831,19 +831,19 @@ nsEventStateManager::PreHandleEvent(nsPr @@ -3985,28 +3987,31 @@ nsEventStateManager::SendFocusBlur(nsPre @@ -4040,21 +4045,22 @@ nsEventStateManager::SendFocusBlur(nsPre
(In reply to comment #25) > >+ : nsDOMEvent(aPresContext, aEvent ? > >+ NS_STATIC_CAST(nsEvent *, aEvent) : > >+ NS_STATIC_CAST(nsEvent *, new nsUIEvent(PR_FALSE, 0, 0))) > > Are these casts needed? Yeha, w/o the casts I get: ../../../../mozilla/content/events/src/nsDOMUIEvent.cpp:58: error: conditional expression between distinct pointer types `nsGUIEvent*' and `nsUIEvent*' lacks a cast Fixed the rest, including sickings latest findings (except the nsHTMLLableElement case which we decided should remain as is).
Comment on attachment 181923 [details] [diff] [review] Same thing, but fix CreateEvent() users too, and remove unused MouseClicked() methods in some layout code, and implement nsIDOMNSEventTarget::AddEventListener() on all targets. a=chofmann
Attachment #181923 - Flags: approval1.8b2? → approval1.8b2+
Attached patch Patch that's being checked in. (deleted) — Splinter Review
Patch checked in on the trunk.
Comment on attachment 182207 [details] [diff] [review] fix for qt build bustage (checked in 2005-04-29 15:29 PDT) r+sr=jst
Attachment #182207 - Flags: superreview?(jst)
Attachment #182207 - Flags: superreview+
Attachment #182207 - Flags: review?(jst)
Attachment #182207 - Flags: review+
Attachment #182207 - Attachment description: fix for qt build bustage → fix for qt build bustage (checked in 2005-04-29 15:29 PDT)
Comment on attachment 182207 [details] [diff] [review] fix for qt build bustage (checked in 2005-04-29 15:29 PDT) a=asa
Attachment #182207 - Flags: approval1.8b2+
Marking FIXED.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Depends on: 292326
Depends on: 292464
Blocks: 294323
*** Bug 294323 has been marked as a duplicate of this bug. ***
Depends on: 295210
Blocks: 289236
Blocks: 297038
Flags: blocking1.7.9+
Flags: blocking-aviary1.0.5+
jst: Is the Trunk patch safe for the branch? If so, let's get this checked in for 1.0.5. Otherwise, let's get a new patch with reviews.
Whiteboard: [sg:fix] needed for b2 cuz it breaks things → [sg:fix] need branch landing , needed for b2 cuz it breaks things
Jay, dveditz is working on porting the patch to the branch.
Attachment #181474 - Flags: review?(bugmail)
Attached patch preliminary aviary patch (obsolete) (deleted) — Splinter Review
This is a preliminary aviary-branch patch. Still making a couple tweaks
jst: nsXBLPrototypeHandler::BindingAttached and nsXBLPrototypeHandler::BindingDetached call CreateEvent -- should those have the trusted bits set?
dveditz, yeah, both of those should fire trusted events.
Status: RESOLVED → VERIFIED
(In reply to comment #40) > Created an attachment (id=186648) > This is a preliminary aviary-branch patch. Still making a couple tweaks I've completed the patch and incorporated bug 292326 and bug 292464 (295210 was checked in independently). I'm not attaching it because it doesn't actually work -- the "blocked" bugs are still exploitable on the branch, for example. Diffing against the trunk now to see what I flipped the wrong way. Could be my replacement for the trunk's GetOwnerDoc() was wrong :-( (nsGenericElement and a couple others).
Attached patch Aviary branch patch (deleted) — Splinter Review
Found the missing lines doing a careful diff of diffs between the trunk and branch patches. Patch now does what it's supposed to.
Attachment #186648 - Attachment is obsolete: true
Attachment #186722 - Flags: superreview?(bugmail)
Attachment #186722 - Flags: review?(jst)
Looks like the new file nsIDOMNSEventTarget.idl is missing from the diff (should be able to just copy that from the trunk).
The implementation of the method nsContentUtils::IsChromeDoc() is missing a return value type declaration, it should look like: PRBool nsContentUtils::IsChromeDoc(nsIDocument *aDocument)
(In reply to comment #45) > Looks like the new file nsIDOMNSEventTarget.idl is missing Forgot -N on the diff, it's in there though. (In reply to comment #46) > The implementation of the method nsContentUtils::IsChromeDoc() is missing a > return value type declaration Thanks. I'm going to check this in so QA can start beating on tomorrow's builds.
Fix checked in to mozilla 1.7 and aviary101 branches.
Blocks: 289192
Blocks: 296704
Adding distributors
Security advisories published
Group: security
Maybe the Bug 300852 is related to this one?
This is new MFSA2005-45 http://www.mozilla.org/security/announce/mfsa2005-45.html and SA16043's vulnerability #1; see http://secunia.com/advisories/16043/ . Alias field is not yet updated to contain SA16043.
This patched disabled ability the catch to key events and changing them to another one, which as I far as I know can only be done using initkeyevent, check the second example on this page: http://www.pooyak.com/blog/archives/000146.shtml I know that this is something that potentially can cause many security problems but at the same time it is something very useful for web application. For example a popular usage is to make the tab key working in a text area, as I did on the edit page of Uniwakka, a scientific Wiki. I remember the old version of Yahoo Mail also had functionality (IE Only), but I couldn’t find a reference to it. There tab is used for indentation: http://www.istitutocolli.org/uniwakka/SandBox/edit I also use that to port a popular script for changing the keyboard layout to Firefox. This script allows users to type in Persian language in text inputs without having any special software installed. The IE version of it is very popular in Persian community: http://www.pooyak.com/p/persianjavascript/ In Internet Explorer it is done by either changing keyCode property of the event (which as I remember is read-only in Mozilla) or using some IE specific text selection functions to find the cursor position and put the new character there. I'm looking forward either for getting back this functionality or knowing alternate solutions to it. Thanks More info on request: me at pooyak.com
Depends on: 305120
No longer depends on: 305120
Could this have caused Bug 305970 ? It falls right into the timeframe that bug occured.
Comment on attachment 186722 [details] [diff] [review] Aviary branch patch Resetting old request flags
Attachment #186722 - Flags: superreview?(bugmail)
Attachment #186722 - Flags: review?(jst)
This caused bug 303713
Depends on: 303713
Depends on: 323251
Depends on: 321831
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: