crash initialising iframe as html edit where html loaded contains a second iframe [@ nsQueryInterface::operator()]

VERIFIED FIXED

Status

()

Core
Editor
--
critical
VERIFIED FIXED
11 years ago
11 years ago

People

(Reporter: Nick Mott, Assigned: sicking)

Tracking

(5 keywords)

Trunk
x86
Windows XP
crash, fixed1.8.1, testcase, verified1.8.0.3, verified1.8.0.4
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8.0.3 +
blocking1.8.0.4 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [camino-1.0.1], crash signature)

Attachments

(6 attachments, 4 obsolete attachments)

765 bytes, text/html
Details
5.34 KB, patch
dbaron
: review+
dbaron
: superreview+
Details | Diff | Splinter Review
5.28 KB, patch
Details | Diff | Splinter Review
5.59 KB, patch
Details | Diff | Splinter Review
5.52 KB, patch
Details | Diff | Splinter Review
6.25 KB, patch
Details | Diff | Splinter Review
(Reporter)

Description

11 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.2) Gecko/20060308 Firefox/1.5.0.2
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.2) Gecko/20060308 Firefox/1.5.0.2

Setting an iframe as an html edit frame and loading html content which contains a second iframe crashes Firefox 1.5.0.2 (have not tried other versions)

The content seems to load, but then the iframe content disappears. Clicking on the iframe then causes Firefox to crash.



Reproducible: Always

Steps to Reproduce:
1. Create a html page with the following code:

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<title>Untitled Document</title>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<script>
function initIframe(){
var html;
html = "<html><head></head><body>Hello World!<br><iframe src=\"http://www.mozilla.org\">&nbsp;</iframe></body></html>";
var RTE_example = document.getElementById("example");
RTE_example.contentWindow.document.designMode = "on";
RTE_example.contentWindow.document.open();
RTE_example.contentWindow.document.write(html);
RTE_example.contentWindow.document.close();
};
</script>
</head>
<body onLoad="javascript: initIframe();">
<iframe id="example"><html><head></head><body>&nbsp;</body></html></iframe>
</body>
</html>

2. load the page
3. click on the iframe

Actual Results:  
the expected content is not displayed and Firefox crahses.

Expected Results:  
an editable iframe should have been produced.
Created attachment 218865 [details]
reporter's testcase
(Reporter)

Comment 2

11 years ago
Created attachment 218871 [details]
updated test case

The previous test test missed a space between 'iframe' and 'src'.
(Reporter)

Comment 3

11 years ago
Created attachment 218873 [details]
details of the crash

Comment 4

11 years ago
Could you please install Talkback and get a Talkback ID for the crash? http://kb.mozillazine.org/Talkback

Comment 5

11 years ago
No need for talkback id.

xpcom_core.dll!nsQueryInterface::operator()(const nsID & aIID={...}, void * * answer=0x0012c368)  Line 47
editor.dll!nsCOMPtr<nsIEditor>::assign_from_qi(nsQueryInterface qi={...}, const nsID & aIID={...})  Line 1232
editor.dll!nsCOMPtr<nsIEditor>::nsCOMPtr<nsIEditor>(nsQueryInterface qi={...})  Line 646
editor.dll!nsUndoCommand::IsCommandEnabled(const char * aCommandName=0x04663148, nsISupports * aCommandRefCon=0x0483afe0, int * outCmdEnabled=0x0012c564)  Line 74
embedcomponents.dll!nsControllerCommandTable::IsCommandEnabled(const char * aCommandName=0x04663148, nsISupports * aCommandRefCon=0x0483afe0, int * aResult=0x0012c564)  Line 138
embedcomponents.dll!nsBaseCommandController::IsCommandEnabled(const char * aCommand=0x04663148, int * aResult=0x0012c564)  Line 118
xpcom_core.dll!XPTC_InvokeByIndex(nsISupports * that=0x00000003, unsigned int methodIndex=2, unsigned int paramCount=1230164, nsXPTCVariant * params=0x00000000)  Line 102
xpc3250.dll!XPCWrappedNative::CallMethod(XPCCallContext & ccx={...}, XPCWrappedNative::CallMode mode=3)  Line 2155
xpc3250.dll!XPCWrappedNative::CallMethod(XPCCallContext & ccx={...}, XPCWrappedNative::CallMode mode=CALL_METHOD)  Line 2155
xpc3250.dll!XPC_WN_CallMethod(JSContext * cx=0x03b47308, JSObject * obj=0x04442aa0, unsigned int argc=1, long * argv=0x048cda84, long * vp=0x0012c818)  Line 1444
js3250.dll!js_Invoke(JSContext * cx=0x03b47308, unsigned int argc=1, unsigned int flags=0)  Line 1246
js3250.dll!js_Interpret(JSContext * cx=0x03b47308, unsigned char * pc=0x03a87760, long * result=0x0012d2cc)  Line 3902
js3250.dll!js_Invoke(JSContext * cx=0x03b47308, unsigned int argc=1, unsigned int flags=2)  Line 1270
js3250.dll!js_InternalInvoke(JSContext * cx=0x03b47308, JSObject * obj=0x044ce850, long fval=73556936, unsigned int flags=0, unsigned int argc=1, long * argv=0x0012d558, long * rval=0x0012d560)  Line 1347
js3250.dll!JS_CallFunctionValue(JSContext * cx=0x03b47308, JSObject * obj=0x044ce850, long fval=73556936, unsigned int argc=1, long * argv=0x0012d558, long * rval=0x0012d560)  Line 4186
gklayout.dll!nsJSContext::CallEventHandler(JSObject * aTarget=0x044ce850, JSObject * aHandler=0x046263c8, unsigned int argc=1, long * argv=0x0012d558, long * rval=0x0012d560)  Line 1426
gklayout.dll!nsJSEventListener::HandleEvent(nsIDOMEvent * aEvent=0x0485a690)  Line 186
gklayout.dll!nsEventListenerManager::HandleEventSubType(nsListenerStruct * aListenerStruct=0x0440ba00, nsIDOMEventListener * aListener=0x0440b958, nsIDOMEvent * aDOMEvent=0x0485a690, nsISupports * aCurrentTarget=0x0440b8a0, unsigned int aSubType=32, unsigned int aPhaseFlags=6)  Line 1636
gklayout.dll!nsEventListenerManager::HandleEvent(nsPresContext * aPresContext=0x00cd06e0, nsEvent * aEvent=0x0012d9c4, nsIDOMEvent * * aDOMEvent=0x0012d77c, nsISupports * aCurrentTarget=0x0440b8a0, unsigned int aFlags=6, nsEventStatus * aEventStatus=0x0012d780)  Line 1740
gklayout.dll!nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor & aVisitor={...}, unsigned int aFlags=6)  Line 335
gklayout.dll!nsEventTargetChainItem::HandleEventTargetChain(nsEventChainPostVisitor & aVisitor={...}, unsigned int aFlags=6, nsDispatchingCallback * aCallback=0x00000000)  Line 455
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 401
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventDispatcher::Dispatch(nsISupports * aTarget=0x0440b8a0, nsPresContext * aPresContext=0x00cd06e0, nsEvent * aEvent=0x0012d9c4, nsIDOMEvent * aDOMEvent=0x00000000, nsEventStatus * aEventStatus=0x0012d9c0, nsDispatchingCallback * aCallback=0x00000000, int aTargetIsChromeHandler=0)  Line 575
gklayout.dll!nsXULCommandDispatcher::UpdateCommands(const nsAString_internal & aEventName={...})  Line 415
gklayout.dll!nsGlobalWindow::UpdateCommands(const nsAString_internal & anAction={...})  Line 4472
gklayout.dll!nsFocusController::UpdateCommands()  Line 217
gklayout.dll!nsFocusController::Focus(nsIDOMEvent * aEvent=0x04637cf0)  Line 362
gklayout.dll!DispatchToInterface(nsIDOMEvent * aEvent=0x04637cf0, nsIDOMEventListener * aListener=0x03b4714c, unsigned int (nsIDOMEvent *)* aMethod=0x0187ce50, const nsID & aIID={...}, int * aHasInterface=0x0012dc34)  Line 143
gklayout.dll!nsEventListenerManager::HandleEvent(nsPresContext * aPresContext=0x04892528, nsEvent * aEvent=0x0012e388, nsIDOMEvent * * aDOMEvent=0x0012dd18, nsISupports * aCurrentTarget=0x03a83454, unsigned int aFlags=4, nsEventStatus * aEventStatus=0x0012dd1c)  Line 1730
gklayout.dll!nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor & aVisitor={...}, unsigned int aFlags=4)  Line 335
gklayout.dll!nsEventTargetChainItem::HandleEventTargetChain(nsEventChainPostVisitor & aVisitor={...}, unsigned int aFlags=6, nsDispatchingCallback * aCallback=0x00000000)  Line 429
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 401
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventTargetChainItem::CreateChainAndHandleEvent(nsEventChainPreVisitor & aVisitor={...}, nsDispatchingCallback * aCallback=0x00000000)  Line 392
gklayout.dll!nsEventDispatcher::Dispatch(nsISupports * aTarget=0x048d14f8, nsPresContext * aPresContext=0x04892528, nsEvent * aEvent=0x0012e388, nsIDOMEvent * aDOMEvent=0x00000000, nsEventStatus * aEventStatus=0x0012e384, nsDispatchingCallback * aCallback=0x00000000, int aTargetIsChromeHandler=0)  Line 575
gklayout.dll!nsEventStateManager::PreHandleEvent(nsPresContext * aPresContext=0x04892528, nsEvent * aEvent=0x0012e6f8, nsIFrame * aTargetFrame=0x049376bc, nsEventStatus * aStatus=0x0012e50c, nsIView * aView=0x03b58228)  Line 639
gklayout.dll!PresShell::HandleEventInternal(nsEvent * aEvent=0x0012e6f8, nsIView * aView=0x03b58228, nsEventStatus * aStatus=0x0012e50c)  Line 6120
gklayout.dll!PresShell::HandleEvent(nsIView * aView=0x03b58228, nsGUIEvent * aEvent=0x0012e6f8, nsEventStatus * aEventStatus=0x0012e50c)  Line 5901
gklayout.dll!nsViewManager::HandleEvent(nsView * aView=0x03b58228, nsPoint aPoint={...}, nsGUIEvent * aEvent=0x0012e6f8, int aCaptured=0)  Line 1712
gklayout.dll!nsViewManager::DispatchEvent(nsGUIEvent * aEvent=0x0012e6f8, nsEventStatus * aStatus=0x0012e634)  Line 1665
gklayout.dll!HandleEvent(nsGUIEvent * aEvent=0x0012e6f8)  Line 174
gkwidget.dll!nsWindow::DispatchEvent(nsGUIEvent * event=0x0012e6f8, nsEventStatus & aStatus=nsEventStatus_eIgnore)  Line 1100
gkwidget.dll!nsWindow::DispatchWindowEvent(nsGUIEvent * event=0x0012e6f8)  Line 1121
gkwidget.dll!nsWindow::DispatchFocus(unsigned int aEventType=105, int isMozWindowTakingFocus=1)  Line 6215
gkwidget.dll!nsWindow::ProcessMessage(unsigned int msg=7, unsigned int wParam=4981478, long lParam=0, long * aRetValue=0x0012eaf8)  Line 4752
gkwidget.dll!nsWindow::WindowProc(HWND__ * hWnd=0x002f0566, unsigned int msg=7, unsigned int wParam=4981478, long lParam=0)  Line 1289
user32.dll!77d48734()
[Frames below may be incorrect and/or missing, no symbols loaded for user32.dll]
user32.dll!77d48816()
user32.dll!77d4b4c0()
user32.dll!77d4b50c()
ntdll.dll!7c90eae3()
user32.dll!77d4da6c()
gkwidget.dll!nsWindow::SetFocus(int aRaise=1)  Line 2156
gklayout.dll!nsEventStateManager::SendFocusBlur(nsPresContext * aPresContext=0x04892528, nsIContent * aContent=0x00000000, int aEnsureWindowHasFocus=1)  Line 4291
gklayout.dll!nsEventStateManager::SetContentState(nsIContent * aContent=0x00000000, int aState=2)  Line 3877
gklayout.dll!nsEventStateManager::PostHandleEvent(nsPresContext * aPresContext=0x04892528, nsEvent * aEvent=0x0012f654, nsIFrame * aTargetFrame=0x04950874, nsEventStatus * aStatus=0x0012f418, nsIView * aView=0x03b58228)  Line 1944
gklayout.dll!PresShell::HandleEventInternal(nsEvent * aEvent=0x0012f654, nsIView * aView=0x03b58228, nsEventStatus * aStatus=0x0012f418)  Line 6148
gklayout.dll!PresShell::HandlePositionedEvent(nsIView * aView=0x03b58228, nsIFrame * aTargetFrame=0x04950874, nsGUIEvent * aEvent=0x0012f654, nsEventStatus * aEventStatus=0x0012f418)  Line 6005
gklayout.dll!PresShell::HandleEvent(nsIView * aView=0x047eb720, nsGUIEvent * aEvent=0x0012f654, nsEventStatus * aEventStatus=0x0012f418)  Line 5826
gklayout.dll!nsViewManager::HandleEvent(nsView * aView=0x047eb720, nsPoint aPoint={...}, nsGUIEvent * aEvent=0x0012f654, int aCaptured=0)  Line 1712
gklayout.dll!nsViewManager::DispatchEvent(nsGUIEvent * aEvent=0x0012f654, nsEventStatus * aStatus=0x0012f540)  Line 1665
gklayout.dll!HandleEvent(nsGUIEvent * aEvent=0x0012f654)  Line 174
gkwidget.dll!nsWindow::DispatchEvent(nsGUIEvent * event=0x0012f654, nsEventStatus & aStatus=nsEventStatus_eIgnore)  Line 1100
gkwidget.dll!nsWindow::DispatchWindowEvent(nsGUIEvent * event=0x0012f654)  Line 1121
gkwidget.dll!nsWindow::DispatchMouseEvent(unsigned int aEventType=302, unsigned int wParam=1, long lParam=393222)  Line 6095
gkwidget.dll!ChildWindow::DispatchMouseEvent(unsigned int aEventType=302, unsigned int wParam=1, long lParam=393222)  Line 6278
gkwidget.dll!nsWindow::ProcessMessage(unsigned int msg=513, unsigned int wParam=1, long lParam=393222, long * aRetValue=0x0012fac0)  Line 4568
gkwidget.dll!nsWindow::WindowProc(HWND__ * hWnd=0x001f0630, unsigned int msg=513, unsigned int wParam=1, long lParam=393222)  Line 1289
user32.dll!77d48734()
user32.dll!77d48816()
user32.dll!77d489cd()
user32.dll!77d49402()
user32.dll!77d48a10()
gkwidget.dll!nsAppShell::Run()  Line 135
tkitcmps.dll!nsAppStartup::Run()  Line 161
xul.dll!XRE_main(int argc=3, char * * argv=0x00ac7458, const nsXREAppData * aAppData=0x004036b0)  Line 2364
firefox.exe!main(int argc=3, char * * argv=0x00ac7458)  Line 61
firefox.exe!__tmainCRTStartup()  Line 586
firefox.exe!mainCRTStartup()  Line 403
kernel32.dll!7c816d4f()
kernel32.dll!7c8399f3()
Keywords: crash
Summary: Firefox crashes when initialising iframe as html edit where html loaded contains a second iframe → Firefox crashes when initialising iframe as html edit where html loaded contains a second iframe [@ nsQueryInterface::operator()]
Version: unspecified → Trunk

Comment 6

11 years ago
The closest bug I could find was bug 299676.

Confirming using a trunk build from ~16-th.
Assignee: nobody → mozeditor
Status: UNCONFIRMED → NEW
Component: General → Editor
Ever confirmed: true
Keywords: testcase
Product: Firefox → Core
QA Contact: general
Summary: Firefox crashes when initialising iframe as html edit where html loaded contains a second iframe [@ nsQueryInterface::operator()] → crash initialising iframe as html edit where html loaded contains a second iframe [@ nsQueryInterface::operator()]

Updated

11 years ago
Attachment #218865 - Attachment is obsolete: true

Updated

11 years ago
Attachment #218873 - Attachment is obsolete: true

Updated

11 years ago
Flags: blocking1.9a1?

Updated

11 years ago
Assignee: mozeditor → bugmail
Created attachment 219692 [details] [diff] [review]
Patch to fix

This fixes the crash. I no longer crash from the testcases in this bug, bug 331981 and bug 211348.

I still see very strange behaviour from the testcase in bug 331981, but at least the security issue seems to be plugged.
Attachment #219692 - Flags: superreview?(bzbarsky)
Attachment #219692 - Flags: review?(bzbarsky)
Attachment #219692 - Flags: superreview?(bzbarsky)
Attachment #219692 - Flags: superreview+
Attachment #219692 - Flags: review?(bzbarsky)
Attachment #219692 - Flags: review+
Attachment #219692 - Flags: approval-branch-1.8.1+
Comment on attachment 219692 [details] [diff] [review]
Patch to fix

sr=me, fwiw.  Thanks for doing this!
Attachment #219692 - Flags: approval1.8.0.3?
So this patch should be very safe to land with a minimal risk of regression. The patch basically makes us use safe mechanisms for avoiding access to deleted objects, rather than relying on that users clear out the pointer manually.

The only downside to the patch is that not all objects can be used as context, only objects that supports weak references. However all the objects that we use today already do so it should be no problem for existing code.

I havn't investigated yet how exploitable this crash is. I'll do that tomorrow.
Blocks: 211348, 331981
Flags: blocking1.8.0.3?
Created attachment 219775 [details] [diff] [review]
1.8 branch patch (includes bug 332132)

The trunk patch didn't apply cleanly to the 1.8 branch because bug 332132 recently landed on trunk, fixing an OOM crash and changing some whitespace. This patch applies cleanly and I went ahead and added the null checks from 332132.
Attachment #219775 - Flags: review?(bugmail)
Attachment #219775 - Flags: approval-branch-1.8.1?(bzbarsky)
Attachment #219775 - Flags: approval-branch-1.8.1?(bzbarsky) → approval-branch-1.8.1?(dbaron)
Attachment #219775 - Flags: approval-branch-1.8.1?(dbaron) → approval-branch-1.8.1+
Comment on attachment 219775 [details] [diff] [review]
1.8 branch patch (includes bug 332132)

Btw, this is already landed on the 1.8 branch. Still needs to go on the 1.8.0.3 branch
Attachment #219775 - Flags: review?(bugmail)
Attachment #219775 - Flags: review+
Attachment #219775 - Flags: approval1.8.0.3?
Attachment #219775 - Flags: approval-branch-1.8.1+
Keywords: fixed1.8.1
Attachment #219692 - Flags: approval1.8.0.3?
Created attachment 219799 [details] [diff] [review]
Minimal patch for the 1.8.0 branch

This is a minimal patch for the 1.8.0 branch that only includes the parts neccesary to fix the crash.

(This is also what I landed yesterday on the 1.8.1 branch)
Attachment #219775 - Flags: approval1.8.0.3?
Flags: blocking1.9a1?
Comment on attachment 219799 [details] [diff] [review]
Minimal patch for the 1.8.0 branch

approved for the 1.8.0 branch, a=dveditz for drivers.
Attachment #219799 - Flags: approval1.8.0.3+

Comment 14

11 years ago
Comment on attachment 219799 [details] [diff] [review]
Minimal patch for the 1.8.0 branch

Does this patch still fix the crash in  bugs that the current bug depends on: bug
331981 and bug 211348.  See comment #7 regarding attachment 219692 [details] [diff] [review]
Yes it does
So I'm beginning to wonder if any extensions could depend on this object -- it's got a contractID, so they *could* be using it, which means this *could* break them.

Any thoughts on what, if anything, we should do about that?


For the suite, according to sicking, we apparently also need to do stuff for the editor XBL binding, since editor elements are passed as context by the editor code.
Flags: blocking1.8.0.4?
Flags: blocking1.8.0.4+
Flags: blocking1.8.0.3+
Comment on attachment 219799 [details] [diff] [review]
Minimal patch for the 1.8.0 branch

approved for 1.8.0.3 mini-branch
Attachment #219799 - Flags: approval1.8.0.3+
nsBaseCommandController is not a pseudo-interface: we aren't worrying about changes to concrete classes, just interfaces and pseudo-interfaces such as nsIDocument/nsIContent
I've said this in person to everyone involved, but for the record:

* We are incompatibly changing an API to require nsISupportsWeakReference be implemented by the nsISupports * doodad.

* That fixes a potentially exploitable security bug, good.  Alternative of holding a strong ref looks like a leak hazard.

* An API is a contract, which requires all of our code to comply (or be sued!).

* We therefore ought to take the second patch that Jonas has developed, to fix the editor XBL to comply with the new API.

/be
It is a relatively important semantic change to nsIControllerContext, even though the signature is unchanged.
Speaking of which, we should probably document that change in nsIControllerContext.idl.
I have a patch that changes nsIControllerContext.idl that I used to verify that all internal callers were ok. I'll file a new bug on that and attach the patch there.


I am worried about extensions too, but at the time I don't see another solution than the current one. Unfortunatly the code in question is too unowned :(
Just thought I should note this does not affect Firefox 1.0.x or Suite 1.7.x: Martijn Wargers notes in bug 331981 comment 0:

   This is a regression.
   It doesn't crash in a 2005-04-05 build, it does crash in a 2005-04-06 build,
   so my bet is on bug 283897.

That's a year after the aviary/moz1.7 branches, and I've confirmed that Firefox 1.0.8 is not affected.
So one thing we could do wrt extensions is that if the nsISupports passed in doesn't support nsISupportsWeakReference then we store an nsISupports* like we used to.  This does mean that extensions that use this stuff will suffer from issues like this bug if we fail to unset contexts, of course....

The reason bug 283897 was likely relevant here is that it changed editor's behavior when doing a load in an iframe which is in designMode.  Maybe we're not clearing contexts in that case properly for some reason?  Say we try to work with a window's controllers after it's already dropped them or something?

Comment 25

11 years ago
No crash in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3 with attachment 218871 [details]

Comment 26

11 years ago
The patch in attachment 219775 [details] [diff] [review] also fixes a crash in my company's Pearl Comments extension.
Created attachment 219951 [details] [diff] [review]
Unregress editing
Attachment #219951 - Flags: superreview?(dbaron)
Attachment #219951 - Flags: review?(dbaron)
Attachment #219951 - Flags: approval-branch-1.8.1?(dbaron)
Comment on attachment 219951 [details] [diff] [review]
Unregress editing

You should initialize mCommandContextRawPtr to nsnull in the constructor, and maybe propagate errors from GetWeakReference.  With that, r+sr=dbaron.
Attachment #219951 - Flags: superreview?(dbaron)
Attachment #219951 - Flags: superreview+
Attachment #219951 - Flags: review?(dbaron)
Attachment #219951 - Flags: review+
Attachment #219951 - Flags: approval-branch-1.8.1?(dbaron)
Attachment #219951 - Flags: approval-branch-1.8.1+
Created attachment 219953 [details] [diff] [review]
Unregress editing v2

Fixes review comments
Attachment #219951 - Attachment is obsolete: true
Created attachment 219954 [details] [diff] [review]
Unregress editing, branch version

This one applies to the 1.8.* branches
Attachment #219954 - Flags: approval1.8.0.3?
Created attachment 219958 [details] [diff] [review]
Full combined patch

This is the full combined patch for distributors and private trees and the like.
Comment on attachment 219953 [details] [diff] [review]
Unregress editing v2

post facto a=dveditz for Firefox 1.5.0.3 minibranch, 1.8.0 branch, and 1.8 branch
Attachment #219953 - Flags: approval1.8.0.4+
Attachment #219953 - Flags: approval1.8.0.3+
Attachment #219953 - Flags: approval-branch-1.8.1+
Comment on attachment 219953 [details] [diff] [review]
Unregress editing v2

oops, wrong one.
Attachment #219953 - Flags: approval1.8.0.4+
Attachment #219953 - Flags: approval1.8.0.3+
Attachment #219953 - Flags: approval-branch-1.8.1+
Comment on attachment 219954 [details] [diff] [review]
Unregress editing, branch version

a=dveditz for branch landing (for real this time)
Attachment #219954 - Flags: approval1.8.0.4+
Attachment #219954 - Flags: approval1.8.0.3?
Attachment #219954 - Flags: approval1.8.0.3+
Attachment #219954 - Flags: approval-branch-1.8.1+
Comment on attachment 219775 [details] [diff] [review]
1.8 branch patch (includes bug 332132)

ignore this one, handle bug 332132 elsewhere
Attachment #219775 - Attachment is obsolete: true
Keywords: fixed1.8.0.3, fixed1.8.0.4

Updated

11 years ago
Whiteboard: [camino-1.0.1]
Marking this bug fixed. I'll file separate bugs on remaining work to do on the trunk.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Updated

11 years ago
Blocks: 299676
Verified FIXED using 04-27-16 build of SeaMonkey trunk on Windows XP.
Status: RESOLVED → VERIFIED
This bug has the same root cause as the Firefox DoS that hit the press this past week. Dupe-finding fodder:

MozillaZine:      http://forums.mozillazine.org/viewtopic.php?t=408603
Vuln PoC home:    http://www.securident.com/vuln/ff.txt
Bugtraq posting:  http://www.securityfocus.com/archive/1/431878/30/0/
Secunia:          http://secunia.com/advisories/19802/
Also http://securitytracker.com/alerts/2006/Apr/1015981.html
(Reporter)

Comment 40

11 years ago
Just updated to 1.5.0.3

Firefox no longer crashes, but the iframe still does not function properly. 

The contents of the outer iframe still do not load correctly. I should be able to edit text inside around the embedded iframe.

Nick 
 

Comment 41

11 years ago
This has been assigned to 
http://www.securityfocus.com/bid/17671
http://www.securiteam.com/windowsntfocus/5FP0Q20IBY.html
and
http://www.frsirt.com/english/advisories/2006/1614 (Critical Risk 4/4)
too.
US-CERT VU#866300 at http://www.kb.cert.org/vuls/id/866300 is public as well.
Nick, could you please file a followup bug on that?  Thanks!
the Problem is the function document.close()
if an iframe is in the written html (maybe it closes that content and gif back a editable reference to that)
if the html is later added everthing works fine
Robert (Nick?), please file follow-up bugs for the issues you're seeing (preferably with a testcase), and mention the bug number here.
Blocks: 337040
for "document.close()" with written iframe an behavior discussed
the new created bug is: bug 337040 
(with testcase and description)

Updated

11 years ago
Keywords: fixed1.8.0.3, fixed1.8.0.4 → verified1.8.0.3, verified1.8.0.4
Crash Signature: [@ nsQueryInterface::operator()]
You need to log in before you can comment on or make changes to this bug.