Closed Bug 319683 Opened 19 years ago Closed 18 years ago

mozilla crashes [@ call_enumerate] running a not so special script

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9alpha1

People

(Reporter: bugzilla, Assigned: mrbkap)

References

()

Details

(Keywords: crash, verified1.8.0.2, verified1.8.1, Whiteboard: [rft-dl])

Crash Data

Attachments

(2 files, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051111 Firefox/1.5
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051111 Firefox/1.5

the browser crashes when running the following function:

function crash(){
    function f(){
        var x;
        function g(){
            x=1; //reffere anything here or Moz will not crash.
        }
    }
    
    //apply an object to the __proto__ attribute
    f.__proto__={}; //= [];
    
    //the following call will cause Moz. to crash
    f();
}




Reproducible: Always

Steps to Reproduce:
1. create an html page with a script
2. use the code provided in the details above
3. add a button or an onload atribute to the html page which runs the crash() function 
4. watch mozilla crash

Actual Results:  
the browser crashes with the feedback agend coming up and WinXP displaying a window asking to stop the process.

Expected Results:  
The script should run without any problems.

I do understand that mozilla is using the __proto__ property on objects for it's prototype chain handling. Though, as neither mozilla's JavaScript specs nor the !ECMAScript specs disallow the use of such properties in the user's script using such properties should neither crash the script execution nor crash the entire browser.

The prototype chain handling should not depend on the __proto__ it should rather use an internal object not directly visible to the script. Prototype chain handling could sill be made accessable to the script by providing function (e.g. as functions like setProto(obj, newProto) and {{{getProto(obj)}}) This way overwriting even those functions will neither affect the script nor the prototype chain handling. A script could still implement the __proto__ property by using getters and setters. The advantage would be that the __protot__ object would only exsist in that script and cause no problems to other scripts.
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20051208 Firefox/1.6a1
TB12773818Q

Stack Signature call_enumerate be01b64f 
Product ID FirefoxTrunk 
Build ID 2005120805 
Trigger Time 2005-12-09 05:34:53.0 
Platform Win32 
Operating System Windows 98 4.10 build 67766222 
Module JS3250.DLL + (0001b1b9) 
URL visited  
User Comments Bug 319683 
Since Last Crash 1924 sec 
Total Uptime 1924 sec 
Trigger Reason Access violation 
Source File, Line No. c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsfun.c, line 778 
Stack Trace  

call_enumerate  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsfun.c, line 778]
js_PutCallObject  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsfun.c, line 602]
js_Invoke  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1247]
js_Interpret  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 3756]
js_Invoke  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1231]
js_Interpret  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 3756]
js_Invoke  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1231]
js_InternalInvoke  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1308]
JS_CallFunctionValue  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsapi.c, line 4157]
nsJSContext::CallEventHandler  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/dom/src/base/nsJSEnvironment.cpp, line 1424]
nsJSEventListener::HandleEvent  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/dom/src/events/nsJSEventListener.cpp, line 195]
nsEventListenerManager::HandleEventSubType  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/events/src/nsEventListenerManager.cpp, line 1685]
nsEventListenerManager::HandleEvent  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/events/src/nsEventListenerManager.cpp, line 1792]
nsGenericElement::HandleDOMEvent  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/base/src/nsGenericElement.cpp, line 2196]
nsGenericHTMLElement::HandleDOMEventForAnchors  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/html/content/src/nsGenericHTMLElement.cpp, line 1459]
nsHTMLAnchorElement::HandleDOMEvent  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/html/content/src/nsHTMLAnchorElement.cpp, line 292]
PresShell::HandleEventInternal  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp, line 6027]
PresShell::HandleEventWithTarget  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp, line 5925]
nsEventStateManager::CheckForAndDispatchClick  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/events/src/nsEventStateManager.cpp, line 2971]
nsEventStateManager::PostHandleEvent  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/events/src/nsEventStateManager.cpp, line 1959]
PresShell::HandleEventInternal  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp, line 6081]
PresShell::HandleEvent  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp, line 5863]
nsViewManager::HandleEvent  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/view/src/nsViewManager.cpp, line 2545]
nsViewManager::DispatchEvent  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/view/src/nsViewManager.cpp, line 2237]
HandleEvent  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/view/src/nsView.cpp, line 176]
nsWindow::DispatchEvent  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp, line 1162]
nsWindow::DispatchMouseEvent  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp, line 6007]
ChildWindow::DispatchMouseEvent  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp, line 6213]
nsWindow::WindowProc  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/widget/src/windows/nsWindow.cpp, line 1351]
KERNEL32.DLL + 0x363b (0xbff7363b)
KERNEL32.DLL + 0x24497 (0xbff94497)
0x00d28b6a
Crashes the latest JS debug shell as well:
Assertion failure: obj && prop, at jsfun.c:776
Assignee: nobody → general
Component: General → JavaScript Engine
Product: Firefox → Core
QA Contact: general → general
Version: unspecified → Trunk
Keywords: crash
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: mozilla crashes running a not so special script → mozilla crashes [@ call_enumerate] running a not so special script
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-319683.js,v  <--  regress-319683.js
Flags: testcase+
Attached patch Fix the crash (obsolete) — Splinter Review
This has r=brendan. I'm checking it in now. Nominating for the branches, since this is an extremely trivial null-defense patch.

Note that the variables defined in f() are going to be unusable, but at least we don't crash. This is a real case of "don't do that!" if you want your stuff to work.
Assignee: general → mrbkap
Status: NEW → ASSIGNED
Attachment #210951 - Flags: review+
Attachment #210951 - Flags: branch-1.8.1?(brendan)
Attachment #210951 - Flags: approval1.8.0.2?
Comment on attachment 210951 [details] [diff] [review]
Fix the crash

This could potentially cause crashes.
Attachment #210951 - Attachment is obsolete: true
Attachment #210951 - Flags: review-
Attachment #210951 - Flags: review+
Attachment #210951 - Flags: branch-1.8.1?(brendan)
Attachment #210951 - Flags: approval1.8.0.2?
I'm not _entirely_ sure about the check for obj != pobj, but it seems sane to me.
Attachment #210958 - Flags: review?(brendan)
Comment on attachment 210958 [details] [diff] [review]
Fix the crash and don't cause new ones

Need to OBJ_DROP_PROPERTY if prop && pobj != obj.

/be
Attachment #210958 - Flags: review?(brendan) → review-
Attached patch Yeah, that tooSplinter Review
Attachment #210958 - Attachment is obsolete: true
Attachment #210959 - Flags: review?(brendan)
Comment on attachment 210959 [details] [diff] [review]
Yeah, that too

Looks good, but is vec really something that belongs to an object ("wrong object's vector")? Might re-word to say "slots vector" -- also mention mutable __proto__ and cloned function object trouble?

/be
Attachment #210959 - Flags: review?(brendan) → review+
Fix checked into trunk.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
OS: Windows XP → All
Priority: -- → P1
Hardware: PC → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9alpha
Attachment #210963 - Flags: review+
Attachment #210963 - Flags: branch-1.8.1?(brendan)
Attachment #210963 - Flags: approval1.8.0.2?
Verified FIXED using trunk Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060209 Firefox/1.6a1
Status: RESOLVED → VERIFIED
Attachment #210963 - Flags: approval-branch-1.8.1?(brendan) → approval-branch-1.8.1+
Flags: blocking1.8.0.2+
Comment on attachment 210963 [details] [diff] [review]
What I checked in

approved for 1.8.0 branch, a=dveditz
Attachment #210963 - Flags: approval1.8.0.2? → approval1.8.0.2+
Fix checked into the 1.8 branches.
Whiteboard: [rft-dl]
v ff 1.8.0.1/1.8/1.9 20060302 win/linux/mac
Crash Signature: [@ call_enumerate]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: