The default bug view has changed. See this FAQ.

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

VERIFIED FIXED in mozilla1.8.1

Status

()

Core
JavaScript Engine
P1
critical
VERIFIED FIXED
11 years ago
6 years ago

People

(Reporter: Jesse Ruderman, Assigned: brendan)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla1.8.1
crash, testcase, verified1.8.0.8, verified1.8.1
Points:
---
Bug Flags:
blocking1.8.1 +
blocking1.8.0.8 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?], crash signature)

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

11 years ago
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)
(Assignee)

Comment 1

11 years ago
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
(Assignee)

Updated

11 years ago
Status: NEW → ASSIGNED
Flags: blocking1.8.1?
(Assignee)

Comment 2

11 years ago
Created attachment 236491 [details] [diff] [review]
fix
Attachment #236491 - Flags: review?(mrbkap)
(Assignee)

Comment 3

11 years ago
Comment on attachment 236491 [details] [diff] [review]
fix

Not the right patch...

/be
Attachment #236491 - Attachment is obsolete: true
Attachment #236491 - Flags: review?(mrbkap)
(Reporter)

Updated

11 years ago
Whiteboard: [sg:critical?]
(Assignee)

Comment 4

11 years ago
Created attachment 236505 [details] [diff] [review]
fix

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)
(Assignee)

Comment 5

11 years ago
Created attachment 236517 [details] [diff] [review]
fix, v3

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)

Updated

11 years ago
Flags: blocking1.8.1? → blocking1.8.1+

Updated

11 years ago
Attachment #236517 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 6

11 years ago
Fixed on trunk.

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Assignee)

Updated

11 years ago
Attachment #236517 - Flags: approval1.8.1?

Comment 7

11 years ago
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+

Comment 8

11 years ago
verified fixed 1.9 20060903 windows/mac*/linux
Status: RESOLVED → VERIFIED

Comment 9

11 years ago
Comment on attachment 236517 [details] [diff] [review]
fix, v3

a=schrep for drivers.
Attachment #236517 - Flags: approval1.8.1? → approval1.8.1+
(Assignee)

Comment 10

11 years ago
Fixed on the 1.8 branch.

/be
Keywords: fixed1.8.1

Comment 11

11 years ago
verified fixed 1.8 20060906 windows/mac*/linux
Keywords: fixed1.8.1 → verified1.8.1
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+
(Assignee)

Comment 13

11 years ago
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

Comment 14

11 years ago
verified fixed 1.8.0.8 20060927 windows/mac*/linux
Keywords: fixed1.8.0.8 → verified1.8.0.8
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.