Open Bug 1406905 Opened 7 years ago Updated 2 years ago

Don't use rawObj to access safe getters

Categories

(DevTools :: Console, defect, P3)

defect

Tracking

(Not tracked)

People

(Reporter: Oriol, Unassigned)

References

Details

Open the browser console and run this code:

    Object.setPrototypeOf(document.createElement('div'), null)

Result:

    "ObjectActor.prototype.grip previewer function threw an exception: TypeError: rawObj.attributes is undefined
    Stack: DOMNode@resource://devtools/shared/base-loader.js -> resource://devtools/server/actors/object.js:1842:7
    grip@resource://devtools/shared/base-loader.js -> resource://devtools/server/actors/object.js:143:13
    objectGrip@resource://devtools/shared/base-loader.js -> resource://devtools/server/actors/webconsole.js:482:12
    createValueGrip@resource://devtools/shared/base-loader.js -> resource://devtools/server/actors/object.js:2396:14
    createValueGrip@resource://devtools/shared/base-loader.js -> resource://devtools/server/actors/webconsole.js:431:12
    onEvaluateJS@resource://devtools/shared/base-loader.js -> resource://devtools/server/actors/webconsole.js:1006:20
    onEvaluateJSAsync@resource://devtools/shared/base-loader.js -> resource://devtools/server/actors/webconsole.js:887:20
    onPacket@resource://devtools/shared/base-loader.js -> resource://devtools/server/main.js:1791:15
    send/<@resource://devtools/shared/base-loader.js -> resource://devtools/shared/transport/transport.js:570:13
    exports.makeInfallible/<@resource://devtools/shared/base-loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:109:14
    exports.makeInfallible/<@resource://devtools/shared/base-loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:109:14
    Line: 1842, column: 7"
    [object XULElement]

Normal pages don't have this problem because of Xray protection, but the browser console is privileged and has no Xrays. So properties should not be accessed on rawObj. Instead, the getters should be called manually.
Priority: -- → P3
This would be easy to fix with something like

Object.getOwnPropertyDescriptor(Element.prototype, 'attributes').get.call(rawObj)

However, the code runs in a Sandbox, where Element is not defined. And Cu.importGlobalProperties is so limited.
So I guess the console should create a window object (without opening a new tab), but I don't know if this is possible.
Product: Firefox → DevTools
See Also: → 1410470

(In reply to Oriol Brufau [:Oriol] from comment #1)

However, the code runs in a Sandbox, where Element is not defined. And
Cu.importGlobalProperties is so limited.

I think this was solved in bug 1489301, great!

Another example of why rawObj shouldn't be accessed directly:

  1. Load about:config or similar

  2. Open the web console

  3. Run this code:

    Object.defineProperty(new Event('a'), 'p', {enumerable: true, get: () => alert('pwned!')})
    

You will see the alert. This shouldn't happen.

Some guidelines:

  • (bug 1410470) rawObj instanceof Ci.nsIDOMWindow should be something like Window.isInstance(rawObj), which doesn't iterate the prototype chain.
  • rawObj.location (own property getter) should actually be something like DevToolsUtils.getProperty(obj, "location"), which avoids invoking unsafe getters (could have been altered). I would even add DevToolsUtils.getOwnProperty which wouldn't attempt to iterate the prototype chain.
  • rawObj.isConnected (inherited property getter) should actually be something like Object.getOwnPropertyDescriptor(Node.prototype, "isConnected").get.call(document), which avoids assumptions about the prototype chain (could have been altered) and avoids unsafe getters (could have been altered).
  • rawObj should be renamed to unsafeDereference to make it clearer that it's unsafe.
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.