Missing SAVE_SP_AND_PC in STORE_(NUMBER|INT|UINT)

VERIFIED FIXED

Status

()

Core
JavaScript Engine
VERIFIED FIXED
10 years ago
9 years ago

People

(Reporter: Igor Bukanov, Assigned: Igor Bukanov)

Tracking

({verified1.8.1.13})

Trunk
verified1.8.1.13
Points:
---
Bug Flags:
blocking1.8.1.13 +
wanted1.8.1.x +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?])

Attachments

(2 attachments)

(Assignee)

Description

10 years ago
The macros STORE_(NUMBER|INT|UINT) from jsinterp.c do not call SAVE_SP_AND_PC before calling potentially GC-triggering js_NewDoubleValue. This leads to a GC hazard as demonstrated with the following example:

~/m/ff/mozilla/js/src $ cat ~/m/y.js

var expect = f();

gczeal(2);

var actual = f();

if (expect !== actual)
    throw "Hazard: actual="+actual+" expect="+expect;

function f()
{
    var a = 1e10;
    var b = 2e10;
    var c = 3e10;
    
    return (a*2) * ((b*2) * c); 
}

~/m/ff/mozilla/js/src $ ./Linux_All_DBG.OBJ/js ~/m/y.js
uncaught exception: Hazard: actual=1.44e+42 expect=2.4e+31

Note that so far I was not able to discover a way to get a crash with that. But it can be used to read a memory image of allocated structs.
(Assignee)

Comment 1

10 years ago
Created attachment 300249 [details] [diff] [review]
v1

The patch just adds the missing calls to SAVE_SP_AND_PC.
Attachment #300249 - Flags: review?(brendan)

Updated

10 years ago
Attachment #300249 - Flags: review?(brendan)
Attachment #300249 - Flags: review+
Attachment #300249 - Flags: approval1.9+
(Assignee)

Comment 2

10 years ago
I checked in the patch from comment 1 to the trunk:

http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&branch=HEAD&cvsroot=%252Fcvsroot&date=explicit&mindate=1201671180&maxdate=1201671331&who=igor%25mir2.org
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Assignee)

Updated

10 years ago
Flags: blocking1.8.1.13?
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.13?
Flags: blocking1.8.1.13+
Whiteboard: [sg:critical?]
(Assignee)

Comment 3

10 years ago
Comment on attachment 300249 [details] [diff] [review]
v1

The patch applies to the 181 branch as is.
Attachment #300249 - Flags: approval1.8.1.13?
Comment on attachment 300249 [details] [diff] [review]
v1

approved for 1.8.1.13, a=dveditz for release-drivers
Attachment #300249 - Flags: approval1.8.1.13? → approval1.8.1.13+

Comment 5

10 years ago
Created attachment 304953 [details]
js1_5/extensions/regress-414755.js

Updated

10 years ago
Flags: in-testsuite+

Comment 6

10 years ago
v
Status: RESOLVED → VERIFIED
(Assignee)

Comment 7

9 years ago
I checked in the patch from comment 1 to MOZILLA_1_8_BRANCH:

Checking in jsinterp.c;
/cvsroot/mozilla/js/src/jsinterp.c,v  <--  jsinterp.c
new revision: 3.181.2.95; previous revision: 3.181.2.94
done
Keywords: fixed1.8.1.13

Comment 8

9 years ago
v 1.8.1 linux|mac
Keywords: fixed1.8.1.13 → verified1.8.1.13
Group: security

Comment 9

9 years ago
/cvsroot/mozilla/js/tests/js1_5/extensions/regress-414755.js,v  <--  regress-414755.js
initial revision: 1.1
You need to log in before you can comment on or make changes to this bug.