Base class for proxy-based wrappers implementing full membranes

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: jorendorff, Assigned: gal)

Tracking

Other Branch
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
Created attachment 447852 [details] [diff] [review]
WIP 1 - don't expect too much

The most important thing to read is
  https://developer.mozilla.org/en/XPConnect_security_membranes
which is kind of a hopeful fiction, partly describing the current state of play and partly describing where I think we want to be.

Or just read this fragment of code (attached). Unfortunately it's pre-proxy and overengineered-- the Membrane class is unnecessary. Now that I know what I want, it needs to just be a single AbstractWrapper class with a few pure virtual methods called, say,

  wrap(obj)
  rewrap(obj)
  checkAccess(obj, id)
  doGetProperty(...)
  doSetProperty(...)
  doCall(...)
  doConstruct(...)
  ...

But the guts of jsmembrane.cpp is worth looking at.
(Reporter)

Comment 1

8 years ago
We'll need a JS_SetCurrentCompartment in the constructor of AutoEnterMembrane and another in the destructor. (The "current compartment" of a cx controls which compartment/heap new objects are created in; see [1].)

But Blake convinced me yesterday that we do not need all the stack frame munging for any wrapper type except SJOW. The reason SJOWs "sever the stack" is to foil a specific attack via arguments.callee.caller. The attack works if the less-privileged code appears on the stack above (more recent than) more-privileged code. The only way this can happen is via a SJOW call. XPCNW wrapped functions are never plain JS functions; they always wrap XPCOM methods. It can't happen due to a wrappedjs because only chrome JS can implement COM interfaces.

[1] https://wiki.mozilla.org/JavaScript:SpiderMonkey:GC_Futures - Scroll down to the Compartments API.
(Assignee)

Updated

8 years ago
Assignee: general → gal
(Reporter)

Comment 2

8 years ago
Incomplete thoughts follow.

Another reason not to add a stack frame every time we cross the boundary... maybe... is that some XPCOM getters/setters/methods are designed to be called by callers of all different privilege levels. These are called allAccess properties--holdover stuff from before we had wrappers. The getter/setter itself examines the stack if it needs to know the caller's principals. So if the wrapper pushed a stack frame, that could be Bad. (I think this might be fixable but can't really say.)

This is a reminder that we're still transitioning from stack examination to object-capabilities, not there yet.
(Reporter)

Updated

8 years ago
Blocks: 565735
(Assignee)

Comment 3

8 years ago
We implemented this elsewhere. Closing.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.