Open Bug 1270892 Opened 8 years ago Updated 2 years ago

hasNativeConsoleAPI can have page-visible side-effects

Categories

(DevTools :: Console, defect, P2)

defect

Tracking

(Not tracked)

People

(Reporter: bzbarsky, Unassigned)

Details

This function does:

      // We are very explicitly examining the "console" property of
      // the non-Xrayed object here.
      let console = aWindow.wrappedJSObject.console;

That's all well and good, but what if the page has a getter set up for that property name?  Do we really want to invoke it?

Seems to me like we should do something smarter here.  For example, examining the actual property, seeing whether it's a getter, and if so examining what the getter function is.  Or something.
Might there be some existing method to detect if a property has changed between a wrapper and the wrappedJSObject, that doesn't require running code / examining properties on the page?  I wonder, even if we are smarter about checking for getters if there would still be some way for a page to craft things such that it can execute code.

(In reply to Boris Zbarsky [:bz] from comment #0)
> That's all well and good, but what if the page has a getter set up for that
> property name?  Do we really want to invoke it?

This code will not run with escalated permissions, correct?
Priority: -- → P2
The code will not run with escalated permissions, no.  So this is not a security bug, just a correctness one in the sense that the fact that we do this check is page-observable and can lead to logic bugs in pages in unlikely circumstances.
Don't forget the case of proxies. I'm not sure how to detect them; I think there's no standard way.

I wanted to recommend Debugger.Object for this, but:
1) We don't have proxy detection APIs yet
2) We could enable DebuggeeWouldRun as a real error for things like Debugger.Object.prototype.getOwnPropertyDescriptor, but we don't
> I'm not sure how to detect them

From pure JS poking at untrusted objects?  Or from chrome JS that should be able to use whatever we feel like in terms of debugger APIs and so forth?

I would hope we know aWindow is actually a Window object here.  Given that, it's safe to do Object.getOwnPropertyDescriptor on it, because know aWindow is not a proxy that can intercept basic property lookup.  Then once we have the descriptor we can do whatever we want with the getter in it do decide whether it's the right one.  We totally have APIs for this on the C++ side; if the issue is simply that something needs to be exposed to chrome JS that is not exposed right now, we should simply fix that.
If we know the object in question is not a proxy, then I don't see any problem simply using Object.getOwnPropertyDescriptor.
Product: Firefox → DevTools
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.