Skip to content

Commit

Permalink
Bug 1820951 - Part 1: Implement DevToolsUtils.hasSafeGetter based on …
Browse files Browse the repository at this point in the history
…explicit allowlist and DebuggerObject.prototype.isNativeGetterWithJitInfo. r=nchevobbe,devtools-reviewers

Perform the same test as `nativeIsEagerlyEvaluateable` in `hasSafeGetter`.

Differential Revision: https://phabricator.services.mozilla.com/D175093
  • Loading branch information
arai-a committed May 12, 2023
1 parent 430a9f7 commit 5ef6454
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 7 deletions.
9 changes: 6 additions & 3 deletions devtools/server/actors/webconsole/eager-ecma-allowlist.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ function getter(obj, name) {

const TypedArray = Reflect.getPrototypeOf(Int8Array);

const allowList = [
const functionAllowList = [
Array,
Array.from,
Array.isArray,
Expand Down Expand Up @@ -162,6 +162,9 @@ const allowList = [
isFinite,
isNaN,
unescape,
];

const getterAllowList = [
getter(ArrayBuffer.prototype, "byteLength"),
getter(ArrayBuffer, Symbol.species),
getter(Array, Symbol.species),
Expand Down Expand Up @@ -238,8 +241,8 @@ const changesArrayByCopy = [
];
for (const fn of changesArrayByCopy) {
if (typeof fn == "function") {
allowList.push(fn);
functionAllowList.push(fn);
}
}

module.exports = allowList;
module.exports = { functions: functionAllowList, getters: getterAllowList };
3 changes: 2 additions & 1 deletion devtools/server/actors/webconsole/eval-with-debugger.js
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,8 @@ function ensureSideEffectFreeNatives(maybeEvalGlobal) {
}

const natives = [
...eagerEcmaAllowlist,
...eagerEcmaAllowlist.functions,
...eagerEcmaAllowlist.getters,

// Pull in all of the non-ECMAScript native functions that we want to
// allow as well.
Expand Down
56 changes: 55 additions & 1 deletion devtools/shared/DevToolsUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ ChromeUtils.defineModuleGetter(
"resource://gre/modules/ObjectUtils.jsm"
);

ChromeUtils.defineLazyGetter(lazy, "eagerEcmaAllowlist", () => {
return require("resource://devtools/server/actors/webconsole/eager-ecma-allowlist.js");
});

// Using this name lets the eslint plugin know about lazy defines in
// this file.
var DevToolsUtils = exports;
Expand Down Expand Up @@ -266,6 +270,28 @@ exports.isSafeDebuggerObject = function(obj) {
return true;
};

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

/**
* Generate gSideEffectFreeGetters map.
*/
function ensureSideEffectFreeGetters() {
if (gSideEffectFreeGetters) {
return;
}

const map = new Map();
for (const n of lazy.eagerEcmaAllowlist.getters) {
if (!map.has(n.name)) {
map.set(n.name, []);
}
map.get(n.name).push(n);
}

gSideEffectFreeGetters = map;
}

/**
* Determines if a descriptor has a getter which doesn't call into JavaScript.
*
Expand All @@ -279,7 +305,35 @@ exports.hasSafeGetter = function(desc) {
// unwrapping.
let fn = desc.get;
fn = fn && exports.unwrap(fn);
return fn?.callable && fn?.class == "Function" && fn?.script === undefined;
if (!fn) {
return false;
}
if (!fn.callable || fn.class !== "Function") {
return false;
}
if (fn.script !== undefined) {
// This is scripted function.
return false;
}

// This is a getter with native function.

// We assume all DOM getters have no major side effect, and they are
// eagerly-evaluateable.
//
// JitInfo is used only by methods/accessors in WebIDL, and being
// "a getter with JitInfo" can be used as a condition to check if given
// function is DOM getter.
//
// This includes privileged interfaces in addition to standard web APIs.
if (fn.isNativeGetterWithJitInfo()) {
return true;
}

// Apply explicit allowlist.
ensureSideEffectFreeGetters();
const natives = gSideEffectFreeGetters.get(fn.name);
return natives && natives.some(n => fn.isSameNative(n));
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@
}));
window.varify = true;
const Cu_Sandbox = Cu ? Cu.Sandbox : null;
`;
await evaluateExpression(state.webConsoleFront, script);

Expand Down Expand Up @@ -218,8 +220,8 @@

async function doAutocompleteSandbox(webConsoleFront) {
// Check that completion provides inherited properties even if [[OwnPropertyKeys]] throws.
info("test autocomplete for 'Cu.Sandbox.'");
const response = await webConsoleFront.autocomplete("Cu.Sandbox.");
info("test autocomplete for 'Cu_Sandbox.'");
const response = await webConsoleFront.autocomplete("Cu_Sandbox.");
ok(!response.matchProp, "matchProp");
const keys = Object.getOwnPropertyNames(Object.prototype).sort();
is(response.matches.length, keys.length, "matches.length");
Expand Down

0 comments on commit 5ef6454

Please sign in to comment.