Closed
Bug 522024
Opened 15 years ago
Closed 15 years ago
unexpected TypeError while calling a function
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
status1.9.2 | --- | beta4-fixed |
status1.9.1 | --- | unaffected |
People
(Reporter: soubok, Unassigned)
References
Details
(Keywords: regression, Whiteboard: [sg:moderate] (read from stack location) fixed-in-tracemonkey)
Attachments
(1 file, 1 obsolete file)
1.88 KB,
patch
|
dmandelin
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.3) Gecko/20090824 Firefox/3.5.3
Build Identifier: trunk
The following script reports a TypeError at the 4st loop.
I cannot reproduce the issue in the js.exe shell.
var list = [];
function Add() {
list.push(arguments);
}
function Run() {
for each ( var item in list )
item[0](); // x!x!x!debug.js:10: TypeError: item[0] is not a function
}
for ( var i = 0; i < 10; i++ ) {
Add(function(s) { print('x!') });
}
Run();
Reproducible: Always
If I remove JSOPTION_JIT or JSOPTION_COMPILE_N_GO, the error disappear.
Updated•15 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 2•15 years ago
|
||
Dave, note the list.push(arguments) part. That seems to be involved.
Updated•15 years ago
|
Flags: blocking1.9.2?
Attachment #407900 -
Flags: review?(dmandelin)
Comment 4•15 years ago
|
||
Comment on attachment 407900 [details] [diff] [review]
fix
Brief description for posterity: the bug (introduced by me) was on a path that reads an argument through the arguments object that is stored on trace, but passed to the interpreter, so the value needs to be unboxed. And for that, you need the type. I arranged for the typemap to be accessible to this builtin. I did that, but I forgot to replace prototype code that used a fixed type of 1 (integer).
Attachment #407900 -
Flags: review?(dmandelin) → review+
![]() |
||
Updated•15 years ago
|
Group: core-security
Whoops, as discussed on IRC this wasn't a full fix. Reading from |apn| there is invalid anyway since the trace is no longer active. js_PutArguments has to clear the argsobj's private.
Attachment #407900 -
Attachment is obsolete: true
Attachment #407911 -
Flags: review?(dmandelin)
I marked this security sensitive because it lets you read random memory, albeit tagged as an integer. This can be verified by running the first patch + test case under valgrind where you get an invalid read error.
Updated•15 years ago
|
Attachment #407911 -
Flags: review?(dmandelin) → review+
Whiteboard: fixed-in-tracemonkey
Updated•15 years ago
|
Whiteboard: fixed-in-tracemonkey → [sg:moderate] (read from stack location) fixed-in-tracemonkey
Comment 10•15 years ago
|
||
Status: NEW → RESOLVED
Closed: 15 years ago
Flags: blocking1.9.2? → blocking1.9.2+
Resolution: --- → FIXED
Comment 11•15 years ago
|
||
status1.9.2:
--- → final-fixed
Comment 14•15 years ago
|
||
(In reply to comment #4)
> Brief description for posterity: the bug (introduced by me) [...]
Is there a specific bug number for the patch that caused the regression? If so please add it to this bug's "Blocks:" list. I'm guessing we have no plans to land whatever it was on older branches (marking 1.9.1 "unaffected" for now), but good to have just in case some future set of bugs needs it as a prerequisite.
status1.9.1:
--- → unaffected
Keywords: regression
Updated•14 years ago
|
Group: core-security
You need to log in
before you can comment on or make changes to this bug.
Description
•