Closed Bug 289940 Opened 19 years ago Closed 19 years ago

Chrome code needs to be protected from untrusted events.

Categories

(Core :: DOM: Events, defect)

Product:
Core
Component:
DOM: Events
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
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)

Make ns*Event ctor's take a "trusted" arg, and make chrome not get untusted events by default (WIP still)
297.52 KB, patch
Details | Diff | Splinter Review
Same thing, but fix CreateEvent() users too, and remove unused MouseClicked() methods in some layout code, and implement nsIDOMNSEventTarget::AddEventListener() on all targets.
371.95 KB, patch
sicking
: review+
peterv
: superreview+
chofmann
: approval1.8b2+
Details | Diff | Splinter Review
Patch that's being checked in.
428.84 KB, patch
Details | Diff | Splinter Review
fix for qt build bustage (checked in 2005-04-29 15:29 PDT)
1.11 KB, patch
jst
: review+
jst
: superreview+
asa
: approval1.8b2+
Details | Diff | Splinter Review
Aviary branch patch
407.33 KB, patch
Details | Diff | Splinter Review 
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
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+
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: 19 years ago
Resolution: --- → FIXED
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) — — 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).
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.
Keywords: fixed-aviary1.0.5, fixed1.7.9
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

Creator:
Created:
Updated:
Size: