ordering problem in script_toSource

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Brian Crowder, Assigned: Brian Crowder)

Tracking

({fixed1.8.0.10, fixed1.8.1.2, regression})

Trunk
fixed1.8.0.10, fixed1.8.1.2, regression
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?] potential memory issue)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

11 years ago
Created attachment 253142 [details] [diff] [review]
check instanceof first

I realized after checking it in that my recent patch introduced an ordering problem that was recommended in a review for a later patch.  Here's the fix.  I landed my broken-ness on both branches, of course.  Yay me.

This may not _itself_ be security-critical but it is a real bug, and is related to patches from security-critical bugs, so I have marked it security sensitive for now.  Bug 367120 is where I originally perpetrated my mayhem.
Attachment #253142 - Flags: review?(brendan)
Attachment #253142 - Flags: approval1.8.1.2?
Attachment #253142 - Flags: approval1.8.0.10?
(Assignee)

Comment 1

11 years ago
Created attachment 253143 [details] [diff] [review]
with context

Sorry for bugspam.
Attachment #253142 - Attachment is obsolete: true
Attachment #253143 - Flags: review?(brendan)
Attachment #253143 - Flags: approval1.8.1.2?
Attachment #253143 - Flags: approval1.8.0.10?
Attachment #253142 - Flags: review?(brendan)
Attachment #253142 - Flags: approval1.8.1.2?
Attachment #253142 - Flags: approval1.8.0.10?

Updated

11 years ago
Attachment #253143 - Flags: review?(brendan) → review+
Comment on attachment 253143 [details] [diff] [review]
with context

a=dveditz for 1.8/1.8.0 branches
Attachment #253143 - Flags: approval1.8.1.2?
Attachment #253143 - Flags: approval1.8.1.2+
Attachment #253143 - Flags: approval1.8.0.10?
Attachment #253143 - Flags: approval1.8.0.10+
(Assignee)

Comment 3

11 years ago
Trunk:
Checking in jsscript.c;
/cvsroot/mozilla/js/src/jsscript.c,v  <--  jsscript.c
new revision: 3.134; previous revision: 3.133
done

Moz-1.8:
Checking in jsscript.c;
/cvsroot/mozilla/js/src/jsscript.c,v  <--  jsscript.c
new revision: 3.79.2.22; previous revision: 3.79.2.21
done

Moz-1.8.0:
Checking in jsscript.c;
/cvsroot/mozilla/js/src/jsscript.c,v  <--  jsscript.c
new revision: 3.79.2.5.2.6; previous revision: 3.79.2.5.2.5
done
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Assignee)

Updated

11 years ago
Keywords: fixed1.8.0.10, fixed1.8.1.2

Updated

11 years ago
Flags: in-testsuite-
(Assignee)

Comment 4

11 years ago
Adding taras to this bug as an example of a potentially statically-analyzable bad bug.  Basically, the ordering mistake here (getting the script pointer too early) allows the value conversion to destroy the referenced heap data, leaving you with a pointer into bogus memory.  I'm not sure if this really IS statically analyzable, but you might be able to posit some "lint-like" rules (the pointer returned by GetPrivate could be considered untrusted after a variety of JS routines run).
Blocks: 367120
Keywords: regression
Whiteboard: [sg:nse]
Whiteboard: [sg:nse] → [sg:critical?] potential memory issue
Group: security
You need to log in before you can comment on or make changes to this bug.