DOM object can be used to circumvent security checks

RESOLVED FIXED in mozilla1.8rc1

Status

()

Core
DOM
P2
normal
RESOLVED FIXED
12 years ago
10 years ago

People

(Reporter: shutdown, Assigned: mrbkap)

Tracking

({fixed1.7.13, fixed1.8})

Trunk
mozilla1.8rc1
x86
Windows 98
fixed1.7.13, fixed1.8
Points:
---
Bug Flags:
blocking1.7.13 +
blocking-aviary1.0.8 +
blocking1.8rc2 +
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical])

Attachments

(2 attachments)

(Reporter)

Description

12 years ago
DOM object wrapper can be used to circumvent
security check for the "constructor" property.
(Reporter)

Comment 1

12 years ago
Created attachment 200436 [details]
testcase

Works on:
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20051021 Firefox/1.6a1
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b5) Gecko/20051021 Firefox/1.5
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7.12) Gecko/20051021 Firefox/1.0.7
Confirming because this works in 1.5b2 and 1.0.x, but it appears to be fixed in today's build ("function Function must be called directly, and not by way of a function of another name"). Is this a dupe of one of the recent ones blake fixed?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking1.7.13+
Flags: blocking-aviary1.0.8+
Whiteboard: [sg:critical] DUPEME
(Assignee)

Comment 3

12 years ago
This was "fixed" by the "belt" part of bug 313236 (i.e., we no longer allow calls to privileged Function constructors from "weak" code). I'm not sure if we want to fix this further up the chain, though. Thoughts are welcome.
Here's what's going on here. We null out HTMLBodyElement, set document.body's prototype to a function from a chrome XBL binding. Then access document.body.constructor. That will resolve the property on the body, and since HTMLBodyElement has been set to null that fails to resolve (which is arguably a bug too, but not this bug), so we keep looking up the proto chain. We find the function, and it's got a constructor property. The JS engine then goes to get that, and sees that it's got a getter, calls the getter and that getter does a checkAccess call in the JS engine. Since this property is being  accessed off of document.body, we call document.body's JSClass' checkAcces hook. That checkAccess hook only protects against accessing certain properties (proto and parent, and watchpoints), and this is just a normal get of a vanilla property, so the hook just returns w/o doing a security check.

Now this could trivially be fixed in the DOM's checkAccess hook, but IMO that's wrong, and probably wouldn't catch all cases of this bug. Seems like if the Function JSClass cares about protecting access to its constructor property (as do all vanilla JS objects), the JS engine needs to call that class' checkAccess hook when a property on an instance of that class is accessed, even if indirectly if the instance is on some other object's prototype chain.

Thoughts?
(Assignee)

Comment 5

12 years ago
Created attachment 201202 [details] [diff] [review]
Fix

This has r=brendan. We need to call the checkAccess hook of the object that we found, not of the object that we're looking on, since if we ended up delegating the resolution, the original object might not care about the access, but the object that we ended up finding the property on might care.
Assignee: general → mrbkap
Status: NEW → ASSIGNED
Attachment #201202 - Flags: superreview?(jst)
Attachment #201202 - Flags: review+
(Assignee)

Comment 6

12 years ago
Comment on attachment 201202 [details] [diff] [review]
Fix

jst sez, "sr=jst".
Attachment #201202 - Flags: superreview?(jst) → superreview+
(Assignee)

Comment 7

12 years ago
Checked into trunk. Tracking to hit rc2 in such an event.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Flags: blocking1.8rc1?
Resolution: --- → FIXED
(Assignee)

Updated

12 years ago
Attachment #201202 - Flags: approval1.8rc1?

Updated

12 years ago
Attachment #201202 - Flags: approval1.7.13?
Attachment #201202 - Flags: approval-aviary1.0.8?
(Assignee)

Updated

12 years ago
Priority: -- → P2
Target Milestone: --- → mozilla1.8rc1

Comment 8

12 years ago
Blake, have we fixed enough of this already with the other bug so that the security implications are remedied?
moving out to the 1.8 rc2 ride-along candidate list. We'll consider taking this if we do an RC2.
Flags: blocking1.8rc1? → blocking1.8rc2?

Updated

12 years ago
Attachment #201202 - Flags: approval1.8rc1? → approval1.8rc2+
(Assignee)

Comment 9

12 years ago
Fix checked into MOZILLA_1_8_BRANCH.
Keywords: fixed1.8
Whiteboard: [sg:critical] DUPEME → [sg:critical]

Updated

12 years ago
Flags: blocking1.8rc2? → blocking1.8rc2+

Updated

11 years ago
Flags: testcase+
Comment on attachment 201202 [details] [diff] [review]
Fix

a=dveditz for drivers, please add fixed-aviary1.0.8 and fixed1.7.13 keywords when checked in.
Attachment #201202 - Flags: approval1.7.13?
Attachment #201202 - Flags: approval1.7.13+
Attachment #201202 - Flags: approval-aviary1.0.8?
Attachment #201202 - Flags: approval-aviary1.0.8+

Comment 11

11 years ago
Comment on attachment 201202 [details] [diff] [review]
Fix

mozilla/js/src/jsobj.c 	3.155.2.20 	MOZILLA_1_7_BRANCH
mozilla/js/src/jsobj.c 	3.155.2.1.4.6.2.12 	AVIARY_1_0_1_20050124_BRANCH

Updated

11 years ago
Keywords: fixed-aviary1.0.8, fixed1.7.13

Comment 12

11 years ago
v.fixed on 1.0.1 Aviary branch with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.13) Gecko/20060213 Firefox/1.0.8, permission denied in testcase.
Keywords: fixed-aviary1.0.8 → verified-aviary1.0.8
Group: security

Updated

10 years ago
Flags: in-testsuite+ → in-testsuite?
You need to log in before you can comment on or make changes to this bug.