Skip to content

Commit

Permalink
Bug 1831819 - Remove per-global function support in eager evaluation …
Browse files Browse the repository at this point in the history 
…allow list. r=devtools-reviewers,nchevobbe

Stop supporting instance methods and getters in allowlist.
Now the allowlist is independent of global and can be shared across multiple
globals.

Differential Revision: https://phabricator.services.mozilla.com/D178884
  • Loading branch information
@arai-a
arai-a committed May 26, 2023
1 parent 92f6cf8 commit bcd1fe1
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 206 deletions.
30 changes: 7 additions & 23 deletions devtools/server/actors/webconsole/eager-function-allowlist.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,11 @@ const idlPureAllowlist = require("resource://devtools/server/actors/webconsole/w

const natives = [];
if (Components.Constructor && Cu) {
// Exclude interfaces only with "instance" property, such as Location,
// which is not available in sandbox.
const props = [];
for (const [iface, ifaceData] of Object.entries(idlPureAllowlist)) {
if ("static" in ifaceData || "prototype" in ifaceData) {
props.push(iface);
}
}

const sandbox = Cu.Sandbox(
Components.Constructor("@mozilla.org/systemprincipal;1", "nsIPrincipal")(),
{
invisibleToDebugger: true,
wantGlobalProperties: props,
wantGlobalProperties: Object.keys(idlPureAllowlist),
}
);

Expand All @@ -31,16 +22,9 @@ if (Components.Constructor && Cu) {
}
}

function collectMethodsAndGetters(obj, methodsAndGetters) {
if ("methods" in methodsAndGetters) {
for (const name of methodsAndGetters.methods) {
maybePush(obj[name]);
}
}
if ("getters" in methodsAndGetters) {
for (const name of methodsAndGetters.getters) {
maybePush(Object.getOwnPropertyDescriptor(obj, name)?.get);
}
function collectMethods(obj, methods) {
for (const name of methods) {
maybePush(obj[name]);
}
}

Expand All @@ -51,7 +35,7 @@ if (Components.Constructor && Cu) {
}

if ("static" in ifaceData) {
collectMethodsAndGetters(ctor, ifaceData.static);
collectMethods(ctor, ifaceData.static);
}

if ("prototype" in ifaceData) {
Expand All @@ -60,9 +44,9 @@ if (Components.Constructor && Cu) {
continue;
}

collectMethodsAndGetters(proto, ifaceData.prototype);
collectMethods(proto, ifaceData.prototype);
}
}
}

module.exports = { natives, idlPureAllowlist };
module.exports = { natives };
84 changes: 10 additions & 74 deletions devtools/server/actors/webconsole/eval-with-debugger.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,7 @@ exports.evalWithDebugger = function (string, options = {}, webConsole) {
const evalString = getEvalInput(string);
const { frame, dbg } = getFrameDbg(options, webConsole);

const { dbgGlobal, bindSelf, evalGlobal } = getDbgGlobal(
options,
dbg,
webConsole
);
const { dbgGlobal, bindSelf } = getDbgGlobal(options, dbg, webConsole);

const helpers = WebConsoleCommandsManager.getWebConsoleCommands(
webConsole,
Expand Down Expand Up @@ -162,7 +158,7 @@ exports.evalWithDebugger = function (string, options = {}, webConsole) {

let noSideEffectDebugger = null;
if (options.eager) {
noSideEffectDebugger = makeSideeffectFreeDebugger(evalGlobal);
noSideEffectDebugger = makeSideeffectFreeDebugger();
}

let result;
Expand Down Expand Up @@ -352,12 +348,10 @@ function forceLexicalInitForVariableDeclarationsInThrowingExpression(
/**
* Creates a side-effect-free debugger instance
*
* @param object maybeEvalGlobal
* If provided, raw debuggee global to get `window` accessors.
* @return object
* Side-effect-free debugger.
*/
function makeSideeffectFreeDebugger(maybeEvalGlobal) {
function makeSideeffectFreeDebugger() {
// We ensure that the metadata for native functions is loaded before we
// initialize sideeffect-prevention because the data is lazy-loaded, and this
// logic can run inside of debuggee compartments because the
Expand All @@ -366,7 +360,7 @@ function makeSideeffectFreeDebugger(maybeEvalGlobal) {
// because building the list of valid native functions is itself a
// side-effectful operation because it needs to populate a
// module cache, among any number of other things.
ensureSideEffectFreeNatives(maybeEvalGlobal);
ensureSideEffectFreeNatives();

// Note: It is critical for debuggee performance that we implement all of
// this debuggee tracking logic with a separate Debugger instance.
Expand Down Expand Up @@ -442,73 +436,18 @@ function makeSideeffectFreeDebugger(maybeEvalGlobal) {
return dbg;
}

exports.makeSideeffectFreeDebugger = makeSideeffectFreeDebugger;

// Native functions which are considered to be side effect free.
let gSideEffectFreeNatives; // string => Array(Function)

/**
* Generate gSideEffectFreeNatives map.
*
* @param object maybeEvalGlobal
* If provided, raw debuggee global to get `window` accessors.
*/
function ensureSideEffectFreeNatives(maybeEvalGlobal) {
function ensureSideEffectFreeNatives() {
if (gSideEffectFreeNatives) {
return;
}

const { natives: domNatives, idlPureAllowlist } = eagerFunctionAllowlist;

const instanceFunctionAllowlist = [];

function collectMethodsAndGetters(obj, methodsAndGetters) {
// This can retrieve xray function if the obj comes from web content.
// Xray function has original native and JitInfo even if the property is
// modified by the web content.

if ("methods" in methodsAndGetters) {
for (const name of methodsAndGetters.methods) {
const func = obj[name];
if (func) {
instanceFunctionAllowlist.push(func);
}
}
}
if ("getters" in methodsAndGetters) {
for (const name of methodsAndGetters.getters) {
const func = Object.getOwnPropertyDescriptor(obj, name)?.get;
if (func) {
instanceFunctionAllowlist.push(func);
}
}
}
}

// `Window` can be undefined if this is off main thread.
if (
maybeEvalGlobal &&
typeof Window === "function" &&
Window.isInstance(maybeEvalGlobal) &&
"Window" in idlPureAllowlist &&
"instance" in idlPureAllowlist.Window
) {
collectMethodsAndGetters(maybeEvalGlobal, idlPureAllowlist.Window.instance);
const maybeLocation = maybeEvalGlobal.location;
if (maybeLocation) {
collectMethodsAndGetters(
maybeLocation,
idlPureAllowlist.Location.instance
);
}
const maybeDocument = maybeEvalGlobal.document;
if (maybeDocument) {
collectMethodsAndGetters(
maybeDocument,
idlPureAllowlist.Document.instance
);
}
}
const { natives: domNatives } = eagerFunctionAllowlist;

const natives = [
...eagerEcmaAllowlist.functions,
Expand All @@ -517,8 +456,6 @@ function ensureSideEffectFreeNatives(maybeEvalGlobal) {
// Pull in all of the non-ECMAScript native functions that we want to
// allow as well.
...domNatives,

...instanceFunctionAllowlist,
];

const map = new Map();
Expand Down Expand Up @@ -634,7 +571,6 @@ function getFrameDbg(options, webConsole) {
* An object that holds the following properties:
* - bindSelf: (optional) the self object for the evaluation
* - dbgGlobal: the global object reference in the debugger
* - evalGlobal: the raw global object
*/
function getDbgGlobal(options, dbg, webConsole) {
let evalGlobal = webConsole.evalGlobal;
Expand All @@ -654,7 +590,7 @@ function getDbgGlobal(options, dbg, webConsole) {
// If we have an object to bind to |_self|, create a Debugger.Object
// referring to that object, belonging to dbg.
if (!options.selectedObjectActor) {
return { bindSelf: null, dbgGlobal, evalGlobal };
return { bindSelf: null, dbgGlobal };
}

// For objects related to console messages, they will be registered under the Target Actor
Expand All @@ -665,18 +601,18 @@ function getDbgGlobal(options, dbg, webConsole) {
webConsole.parentActor.getActorByID(options.selectedObjectActor);

if (!actor) {
return { bindSelf: null, dbgGlobal, evalGlobal };
return { bindSelf: null, dbgGlobal };
}

const jsVal = actor instanceof LongStringActor ? actor.str : actor.rawValue();
if (!isObject(jsVal)) {
return { bindSelf: jsVal, dbgGlobal, evalGlobal };
return { bindSelf: jsVal, dbgGlobal };
}

// If we use the makeDebuggeeValue method of jsVal's own global, then
// we'll get a D.O that sees jsVal as viewed from its own compartment -
// that is, without wrappers. The evalWithBindings call will then wrap
// jsVal appropriately for the evaluation compartment.
const bindSelf = dbgGlobal.makeDebuggeeValue(jsVal);
return { bindSelf, dbgGlobal, evalGlobal };
return { bindSelf, dbgGlobal };
}
134 changes: 57 additions & 77 deletions devtools/server/actors/webconsole/webidl-pure-allowlist.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,100 +8,80 @@

module.exports = {
DOMTokenList: {
prototype: {
methods: ["item", "contains"],
},
prototype: ["item", "contains"],
},
Document: {
prototype: {
methods: [
"getSelection",
"hasStorageAccess",
"getElementsByTagName",
"getElementsByTagNameNS",
"getElementsByClassName",
"getElementById",
"getElementsByName",
"querySelector",
"querySelectorAll",
"createNSResolver",
],
},
prototype: [
"getSelection",
"hasStorageAccess",
"getElementsByTagName",
"getElementsByTagNameNS",
"getElementsByClassName",
"getElementById",
"getElementsByName",
"querySelector",
"querySelectorAll",
"createNSResolver",
],
},
Element: {
prototype: {
methods: [
"getAttributeNames",
"getAttribute",
"getAttributeNS",
"hasAttribute",
"hasAttributeNS",
"hasAttributes",
"closest",
"matches",
"webkitMatchesSelector",
"getElementsByTagName",
"getElementsByTagNameNS",
"getElementsByClassName",
"mozMatchesSelector",
"querySelector",
"querySelectorAll",
"getAsFlexContainer",
"getGridFragments",
"hasGridFragments",
"getElementsWithGrid",
],
},
prototype: [
"getAttributeNames",
"getAttribute",
"getAttributeNS",
"hasAttribute",
"hasAttributeNS",
"hasAttributes",
"closest",
"matches",
"webkitMatchesSelector",
"getElementsByTagName",
"getElementsByTagNameNS",
"getElementsByClassName",
"mozMatchesSelector",
"querySelector",
"querySelectorAll",
"getAsFlexContainer",
"getGridFragments",
"hasGridFragments",
"getElementsWithGrid",
],
},
FormData: {
prototype: {
methods: ["entries", "keys", "values"],
},
prototype: ["entries", "keys", "values"],
},
Headers: {
prototype: {
methods: ["entries", "keys", "values"],
},
prototype: ["entries", "keys", "values"],
},
Node: {
prototype: {
methods: [
"getRootNode",
"hasChildNodes",
"isSameNode",
"isEqualNode",
"compareDocumentPosition",
"contains",
"lookupPrefix",
"lookupNamespaceURI",
"isDefaultNamespace",
],
},
prototype: [
"getRootNode",
"hasChildNodes",
"isSameNode",
"isEqualNode",
"compareDocumentPosition",
"contains",
"lookupPrefix",
"lookupNamespaceURI",
"isDefaultNamespace",
],
},
Performance: {
prototype: {
methods: ["now"],
},
prototype: ["now"],
},
Range: {
prototype: {
methods: [
"isPointInRange",
"comparePoint",
"intersectsNode",
"getClientRects",
"getBoundingClientRect",
],
},
prototype: [
"isPointInRange",
"comparePoint",
"intersectsNode",
"getClientRects",
"getBoundingClientRect",
],
},
Selection: {
prototype: {
methods: ["getRangeAt", "containsNode"],
},
prototype: ["getRangeAt", "containsNode"],
},
URLSearchParams: {
prototype: {
methods: ["entries", "keys", "values"],
},
prototype: ["entries", "keys", "values"],
},
};
Loading

0 comments on commit bcd1fe1

Please sign in to comment.