Constructor window.RegExp does not create valid RegExp object in sandbox

RESOLVED DUPLICATE of bug 1014991

Status

()

RESOLVED DUPLICATE of bug 1014991
4 years ago
4 years ago

People

(Reporter: matthew.gertner, Unassigned)

Tracking

33 Branch
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

4 years ago
I'm using Cu.evalInSandbox with the sandboxPrototype set to a DOM window. In the sandbox I'm running code that uses Lodash (https://github.com/lodash/lodash/blob/master/lodash.js). For whatever reason, it uses window.RegExp explicitly to create a regular expression. This appears to create an object, but when I try to access the `test` method it is undefined. Dumping the object to the console displays "[object Opaque]".

I think that the window.RegExp constructor used inside a sandbox should create a normal RegExp object. I thought this might have something to do with the X-ray wrappers but it doesn't seem to make any different if I set wantXrays to false.
(In reply to Matthew Gertner from comment #0)
> I think that the window.RegExp constructor used inside a sandbox should
> create a normal RegExp object.

It does, but it creates a RegExp that lives in the scope of |window|, because that's what |new Window.RegExp| is supposed to do. The wrapper is opaque because it's an XrayWrapper, but we don't have meaningful Xray traits for RegExp objects, so we default to opaque. Bug 1014991 would fix this - I'm unlikely to have time for it myself anytime soon, but would happily mentor a patch.

> I thought this might have something to do
> with the X-ray wrappers but it doesn't seem to make any different if I set
> wantXrays to false.

That's because your sandbox presumably uses an Expanded Principal, so it gets an XrayWrapper by default.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1014991
(Reporter)

Comment 2

4 years ago
Thanks for the explanation. Considering that I need to get this third-party library working in the sandbox, do you see any possible workaround? I can patch the library but obviously I'd like to avoid that if at all possible.
(Reporter)

Comment 3

4 years ago
BTW I'm not sure if I understand what an Expanded Principal is, but currently I'm using @mozilla.org/systemprincipal;1 as the principal of the sandbox.
> do you see any possible workaround?

Does it work to run this in your sandbox:

  window.RegExp = RegExp;

?  This will set the property on the Xray for the window, so won't affect the page itself, but will make sure Lodash gets the RegExp constructor from the sandbox, not the window.

> but currently I'm using @mozilla.org/systemprincipal;1 as the principal of the sandbox

That will definitely get Xrays.
(In reply to Matthew Gertner from comment #3)
> BTW I'm not sure if I understand what an Expanded Principal is, but
> currently I'm using @mozilla.org/systemprincipal;1 as the principal of the
> sandbox.

Do you actually need it to be system principal? It seems super dangerous to run third-party libraries with full system privileges. What are your goals?
(Reporter)

Comment 6

4 years ago
(In reply to Boris Zbarsky [:bz] from comment #4)
> > do you see any possible workaround?
> 
> Does it work to run this in your sandbox:
> 
>   window.RegExp = RegExp;
> 
> ?  This will set the property on the Xray for the window, so won't affect
> the page itself, but will make sure Lodash gets the RegExp constructor from
> the sandbox, not the window.

Good idea. I'll try that.
(Reporter)

Comment 7

4 years ago
(In reply to Bobby Holley (:bholley) from comment #5)
> Do you actually need it to be system principal? It seems super dangerous to
> run third-party libraries with full system privileges. What are your goals?

It's definitely wrong and it's something I'd like to change, but some brief experimentation made it clear that the effort will be pretty big, so I'm living with it for now. The code that runs in the sandbox is not downloaded from the web and I have a content policy there to prevent arbitrary web content from being downloaded, so it's not as bad as it might seem.
(In reply to Matthew Gertner from comment #7)
> (In reply to Bobby Holley (:bholley) from comment #5)
> > Do you actually need it to be system principal? It seems super dangerous to
> > run third-party libraries with full system privileges. What are your goals?
> 
> It's definitely wrong and it's something I'd like to change, but some brief
> experimentation made it clear that the effort will be pretty big

How come? Is the issue that you need to interact with different origins, or that you need to use Components etc?
(Reporter)

Comment 9

4 years ago
(In reply to Bobby Holley (:bholley) from comment #8)
> How come? Is the issue that you need to interact with different origins, or
> that you need to use Components etc?

I'd have to try it again to remember the exact issues since this was a while ago. Basically I am sending data between different JS contexts and IIRC this was broken pretty badly by changing the principal of the sandbox. It's certainly fixable but it struck me as a non-trivial effort so I've been relunctant to tackle it.
(In reply to Matthew Gertner from comment #9)
> (In reply to Bobby Holley (:bholley) from comment #8)
> > How come? Is the issue that you need to interact with different origins, or
> > that you need to use Components etc?
> 
> I'd have to try it again to remember the exact issues since this was a while
> ago. Basically I am sending data between different JS contexts and IIRC this
> was broken pretty badly by changing the principal of the sandbox. It's
> certainly fixable but it struck me as a non-trivial effort so I've been
> relunctant to tackle it.

I would recommend tackling it to avoid security bugs. Let me know if you run into any questions and I'll do my best to answer them.

See https://developer.mozilla.org/en-US/docs/Components.utils.Sandbox#The_Expanded_Principal , which may be helpful.
(Reporter)

Comment 11

4 years ago
Fair enough. I'll take another crack at it and let you know if I have specific questions.
You need to log in before you can comment on or make changes to this bug.