Closed Bug 412926 Opened 12 years ago Closed 12 years ago

JS_ValueToId(cx, JSVAL_NULL) should return atom for 'null' string

Categories

(Core :: JavaScript Engine, defect, P2)

defect

Tracking

()

VERIFIED FIXED

People

(Reporter: igor, Assigned: igor)

Details

(Keywords: verified1.8.1.13, Whiteboard: [sg:critical?] no sg:critical crash in 1.8.1, fixed by 412340 on trunk,)

Attachments

(2 files)

Consider the following test case:

var obj = { 'null': 1 };

var errors = [];
if (!obj.hasOwnProperty(null))
    errors.push('null property is not owned');
if (!obj.propertyIsEnumerable(null))
    errors.push('null property is not enumerable');

var getter_was_called = false;
obj.__defineGetter__(null, function() { getter_was_called = true; return 1; });
obj['null'];
if (!getter_was_called)
    errors.push('getter was not assigned to the null property')

var setter_was_called = false;
obj.__defineSetter__(null, function() { setter_was_called = true; });
obj['null'] = 2;
if (!setter_was_called)
    errors.push('setter was not assigned to the null property')

if (errors.length)
    throw '\n'+errors.join('\n');

gc();


When I run the test case with the shell I got:
~/m/trunk/mozilla/js/src $ js ~/m/y.js
uncaught exception: 
null property is not owned
null property is not enumerable
getter was not assigned to the null property
setter was not assigned to the null property
segmentation fault

The reason for the exceptions (which also exists on 1.8.1 branch) is that the test case triggers invocation of JS_ValueToId with the id set to null. Currently the function will return it as-is due to the JSVAL_IS_OBJECT check done for XML support when the expected behavior is to return the id for 'null' atom. Hence the test cases will not find the id when looking for the property.

The reason for the segmentation fault (which does *not* exist on 1.8.1 branch) is uncler. In principle the code should tolerate the scope properties with null as an id, but apparently this is not the case. So I mark the bug as sensitive one until I figure out the reason behind the crash.
The reason for the segmentation fault is that jsscope.c uses JSVAL_NULL as a special value for the id of the scope to indicate that the scope belongs to the free list. This is also the case on the branch but so far I was not able to trigger the fault.
I have added the fix to the patch from bug 412340 comment 31. This is the only change between v4 and v5 patches in that bug. Since I can not prove that the bug can not be used for an exploit on 1.8.1 branch, I prefer to embed the fix into a bigger patch to avoid exposure.
Flags: blocking1.9?
Flags: blocking1.8.1.13?
Flags: blocking1.9? → blocking1.9+
Priority: -- → P2
If bug 412340 is marked fixed then is this also fixed on trunk?
Flags: wanted1.8.1.x+
Whiteboard: [sg:critical?]
Can we get a separate branch patch here? Or do we also need bug 412340 on the 1.8 branch?
Flags: blocking1.8.1.13? → blocking1.8.1.13+
(In reply to comment #4)
> Can we get a separate branch patch here? Or do we also need bug 412340 on the
> 1.8 branch?
> 

It is unsafe to port 412340 for the branch. So I will create a small targeted fix.
This has been fixed oo the trunk since the resolution of bug 412340n
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Attached patch fix for 1.8.1Splinter Review
Attachment #303594 - Flags: review?(brendan)
Attachment #303594 - Flags: review?(brendan)
Attachment #303594 - Flags: review+
Attachment #303594 - Flags: approval1.8.1.13?
Whiteboard: [sg:critical?] → [sg:critical?] no sg:critical crash in 1.8.1, fixed by 412340 on trunk,
Comment on attachment 303594 [details] [diff] [review]
fix for 1.8.1

approved for 1.8.1.13, a=dveditz for release-drivers
Attachment #303594 - Flags: approval1.8.1.13? → approval1.8.1.13+
Flags: in-testsuite+
Flags: in-litmus-
v
Status: RESOLVED → VERIFIED
I checked in the patch from comment 7 to MOZILLA_1_8_BRANCH:

Checking in jsapi.c;
/cvsroot/mozilla/js/src/jsapi.c,v  <--  jsapi.c
new revision: 3.214.2.41; previous revision: 3.214.2.40
done
Keywords: fixed1.8.1.13
v 1.8.1 linux|mac
the code touched in the 1.8.1 patch was introduced by js1.7 landing. Thus, I assume that this doesn't apply to 1.8.0 branch. If thats wrong, please request blocking1.8.0.15 again.
Flags: blocking1.8.0.15-
Group: security
/cvsroot/mozilla/js/tests/js1_5/extensions/regress-412926.js,v  <--  regress-412926.js
initial revision: 1.1
You need to log in before you can comment on or make changes to this bug.