Closed Bug 351116 Opened 13 years ago Closed 13 years ago

Crash if formal parameter and inner function have the same name [@ js_DecompileFunction]

Categories

(Core :: JavaScript Engine, defect, P1, critical)

defect

Tracking

()

VERIFIED FIXED
mozilla1.8.1

People

(Reporter: jruderman, Assigned: brendan)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [sg:critical?])

Crash Data

Attachments

(1 file, 2 obsolete files)

javascript:function (s) { function s() { } }

causes a scary crash, at least in a debug build of Firefox:

Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_INVALID_ADDRESS (0x0001) at 0xdadadae2

Thread 0 Crashed:
0   js_DecompileFunction + 996 (jsopcode.c:3258)
1   JS_DecompileFunction + 140 (jsapi.c:4222)
2   js_fun_toString + 2008 (jsfun.c:1476)
3   fun_toString + 68 (jsfun.c:1486)
4   js_Invoke + 4912 (jsinterp.c:1350)
5   js_InternalInvoke + 444 (jsinterp.c:1448)
6   js_TryMethod + 416 (jsobj.c:4534)
7   js_DefaultValue + 112 (jsobj.c:3809)
8   js_ValueToString + 152 (jsstr.c:2681)
9   JS_ValueToString + 40 (jsapi.c:535)
10  JSValueToAString(JSContext*, long, nsAString_internal*, int*) + 116 (nsJSEnvironment.cpp:1160)
11  nsJSContext::EvaluateString(nsAString_internal const&, void*, nsIPrincipal*, char const*, unsigned, unsigned, nsAString_internal*, int*) + 1612 (nsJSEnvironment.cpp:1322)
12  nsJSThunk::EvaluateScript(nsIChannel*) + 2844 (nsJSProtocolHandler.cpp:255)
Fairly old bug, good find.  Not the usual decompiler-doesn't-do-right fluff :-P.

/be
Assignee: general → brendan
OS: Mac OS X 10.4 → All
Priority: -- → P1
Hardware: Macintosh → All
Target Milestone: --- → mozilla1.8.1
Status: NEW → ASSIGNED
Flags: blocking1.8.1?
Attached patch fix (obsolete) — Splinter Review
Attachment #236491 - Flags: review?(mrbkap)
Comment on attachment 236491 [details] [diff] [review]
fix

Not the right patch...

/be
Attachment #236491 - Attachment is obsolete: true
Attachment #236491 - Flags: review?(mrbkap)
Whiteboard: [sg:critical?]
Attached patch fix (obsolete) — Splinter Review
Unlike var, function creates a new property, so we must use SPROP_IS_DUPLICATE to keep the formal parameter property around.

/be
Attachment #236505 - Flags: review?(mrbkap)
Attached patch fix, v3Splinter Review
Just comment fixes, some were quite out of date.

/be
Attachment #236505 - Attachment is obsolete: true
Attachment #236517 - Flags: review?(mrbkap)
Attachment #236505 - Flags: review?(mrbkap)
Flags: blocking1.8.1? → blocking1.8.1+
Attachment #236517 - Flags: review?(mrbkap) → review+
Fixed on trunk.

/be
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Attachment #236517 - Flags: approval1.8.1?
This only crashed for me when I used the javascript: url .
Checking in regress-351116.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-351116.js,v  <--  regress-351116.js
initial revision: 1.1
Flags: in-testsuite+
verified fixed 1.9 20060903 windows/mac*/linux
Status: RESOLVED → VERIFIED
Comment on attachment 236517 [details] [diff] [review]
fix, v3

a=schrep for drivers.
Attachment #236517 - Flags: approval1.8.1? → approval1.8.1+
Fixed on the 1.8 branch.

/be
Keywords: fixed1.8.1
verified fixed 1.8 20060906 windows/mac*/linux
Flags: blocking1.8.0.8?
Flags: blocking1.8.0.8? → blocking1.8.0.8+
Comment on attachment 236517 [details] [diff] [review]
fix, v3

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #236517 - Flags: approval1.8.0.8+
Fixed on the 1.8.0 branch:

Checking in jsopcode.c;
/cvsroot/mozilla/js/src/jsopcode.c,v  <--  jsopcode.c
new revision: 3.89.2.8.2.9; previous revision: 3.89.2.8.2.8
done
Checking in jsparse.c;
/cvsroot/mozilla/js/src/jsparse.c,v  <--  jsparse.c
new revision: 3.142.2.6.2.8; previous revision: 3.142.2.6.2.7
done

/be
Keywords: fixed1.8.0.8
verified fixed 1.8.0.8 20060927 windows/mac*/linux
Am I right saying this is not a problem in the 1.7 branch ? Mozilla complains about a syntax error at the first parenthesis in the testcase...
You can get around the syntax error with

  javascript:x = function (s) { function s() { } }

That makes unfixed 1.8.0.x releases crash, but doesn't crash FF1.0.x.
Group: security
Crash Signature: [@ js_DecompileFunction]
You need to log in before you can comment on or make changes to this bug.