Skip to content

Commit

Permalink
Merge pull request #274 from XmiliaH/fix-272
Browse files Browse the repository at this point in the history 
Fix for non-configurable property access
  • Loading branch information
@patriksimek
patriksimek committed Mar 29, 2020
2 parents 5a5cf0b + cb2e8a3 commit d267a32
Show file tree
Hide file tree
Showing 2 changed files with 208 additions and 44 deletions.
200 changes: 176 additions & 24 deletions lib/contextify.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ local.Reflect.apply = Reflect.apply;
local.Reflect.set = Reflect.set;
local.Reflect.deleteProperty = Reflect.deleteProperty;
local.Reflect.has = Reflect.has;
local.Reflect.defineProperty = Reflect.defineProperty;
local.Reflect.setPrototypeOf = Reflect.setPrototypeOf;
local.Reflect.isExtensible = Reflect.isExtensible;
local.Reflect.preventExtensions = Reflect.preventExtensions;
local.Reflect.getOwnPropertyDescriptor = Reflect.getOwnPropertyDescriptor;

// global is originally prototype of host.Object so it can be used to climb up from the sandbox.
Object.setPrototypeOf(global, Object.prototype);
Expand Down Expand Up @@ -51,18 +56,16 @@ function instanceOf(value, construct) {
try {
return host.Reflect.apply(hasInstance, construct, [value]);
} catch (ex) {
// Never pass the handled expcetion through!
// Never pass the handled exception through!
throw new VMError('Unable to perform instanceOf check.');
// This exception actually never get to the user. It only instructs the caller to return null because we wasn't able to perform instanceOf check.
}
}

const SHARED_ARROW = ()=>{};
function SHARED_FUNC() {}
const SHARED_ARRAY = [];
const SHARED_OBJECT = {__proto__: null};

function getBaseObject(obj) {
function createBaseObject(obj) {
let base;
if (typeof obj === 'function') {
try {
// eslint-disable-next-line no-new
Expand All @@ -72,14 +75,22 @@ function getBaseObject(obj) {
return this;
}
})();
// eslint-disable-next-line func-names
base = function() {};
base.prototype = null;
} catch (e) {
return SHARED_ARROW;
base = () => {};
}
return SHARED_FUNC;
} else if (host.Array.isArray(obj)) {
return SHARED_ARRAY;
base = [];
} else {
return {__proto__: null};
}
if (!local.Reflect.setPrototypeOf(base, null)) {
// Should not happen
return null;
}
return SHARED_OBJECT;
return base;
}

/**
Expand Down Expand Up @@ -109,6 +120,44 @@ function throwCallerCalleeArgumentsAccess(key) {
return new VMError('Unreachable');
}

function unexpected() {
throw new VMError('Should not happen');
}

function doPreventExtensions(target, object, doProxy) {
const keys = local.Reflect.ownKeys(object);
for (let i = 0; i < keys.length; i++) {
const key = keys[i];
let desc = local.Reflect.getOwnPropertyDescriptor(object, key);
if (!desc) continue;
if (!local.Reflect.setPrototypeOf(desc, null)) unexpected();
if (!desc.configurable) {
const current = local.Reflect.getOwnPropertyDescriptor(target, key);
if (current && !current.configurable) continue;
if (desc.get || desc.set) {
desc.get = doProxy(desc.get);
desc.set = doProxy(desc.set);
} else {
desc.value = doProxy(desc.value);
}
} else {
if (desc.get || desc.set) {
desc = {
__proto__: null,
configurable: true,
enumberable: desc.enumberable,
writeable: true,
value: null
};
} else {
desc.value = null;
}
}
if (!local.Reflect.defineProperty(target, key, desc)) unexpected();
}
if (!local.Reflect.preventExtensions(target)) unexpected();
}

/**
* Decontextify.
*/
Expand Down Expand Up @@ -269,25 +318,46 @@ Decontextify.object = (object, traps, deepTraps, flags, mock) => {
// TypeError: 'getOwnPropertyDescriptor' on proxy: trap reported non-configurability for property '<prop>'
// which is either non-existant or configurable in the proxy target

let desc;
if (!def) {
return undefined;
} else if (def.get || def.set) {
return {
desc = {
__proto__: null,
get: Decontextify.value(def.get) || undefined,
set: Decontextify.value(def.set) || undefined,
enumerable: def.enumerable === true,
configurable: def.configurable === true
};
} else {
return {
desc = {
__proto__: null,
value: Decontextify.value(def.value),
writable: def.writable === true,
enumerable: def.enumerable === true,
configurable: def.configurable === true
};
}
if (!desc.configurable) {
try {
def = host.Object.getOwnPropertyDescriptor(target, prop);
if (!def || def.configurable) {
local.Reflect.defineProperty(target, prop, desc);
}
} catch (e) {
// Should not happen.
}
}
return desc;
};
base.defineProperty = (target, key, descriptor) => {
let success = false;
try {
success = local.Reflect.setPrototypeOf(descriptor, null);
} catch (e) {
// Should not happen
}
if (!success) return false;
// There's a chance accessing a property throws an error so we must not access them
// in try catch to prevent contextyfing local objects.

Expand All @@ -305,10 +375,19 @@ Decontextify.object = (object, traps, deepTraps, flags, mock) => {
}

try {
return host.Object.defineProperty(target, key, propertyDescriptor);
success = local.Reflect.defineProperty(object, key, propertyDescriptor);
} catch (e) {
throw Decontextify.value(e);
}
if (success && descriptor.configurable) {
try {
local.Reflect.defineProperty(target, key, descriptor);
} catch (e) {
// This should not happen.
return false;
}
}
return success;
};
base.deleteProperty = (target, prop) => {
try {
Expand All @@ -331,11 +410,22 @@ Decontextify.object = (object, traps, deepTraps, flags, mock) => {
}
};
base.isExtensible = target => {
let result;
try {
return Decontextify.value(local.Object.isExtensible(object));
result = local.Reflect.isExtensible(object);
} catch (e) {
throw Decontextify.value(e);
}
if (!result) {
try {
if (local.Reflect.isExtensible(target)) {
doPreventExtensions(target, object, obj => Contextify.value(obj, null, deepTraps, flags));
}
} catch (e) {
// Should not happen
}
}
return result;
};
base.ownKeys = target => {
try {
Expand All @@ -345,12 +435,22 @@ Decontextify.object = (object, traps, deepTraps, flags, mock) => {
}
};
base.preventExtensions = target => {
let success;
try {
local.Object.preventExtensions(object);
return true;
success = local.Reflect.preventExtensions(object);
} catch (e) {
throw Decontextify.value(e);
}
if (success) {
try {
if (local.Reflect.isExtensible(target)) {
doPreventExtensions(target, object, obj => Contextify.value(obj, null, deepTraps, flags));
}
} catch (e) {
// Should not happen
}
}
return success;
};
base.enumerate = target => {
try {
Expand All @@ -368,6 +468,7 @@ Decontextify.object = (object, traps, deepTraps, flags, mock) => {
shallow = {
__proto__: null,
ownKeys: base.ownKeys,
// TODO this get will call getOwnPropertyDescriptor of target all the time.
get: origGet
};
base.ownKeys = target => {
Expand All @@ -388,9 +489,9 @@ Decontextify.object = (object, traps, deepTraps, flags, mock) => {
shallow = SHARED_OBJECT;
}

const proxy = new host.Proxy(getBaseObject(object), base);
const proxy = new host.Proxy(createBaseObject(object), base);
Decontextified.set(proxy, object);
// We need two proxys since nodes inspect just removes one.
// We need two proxies since nodes inspect just removes one.
const proxy2 = new host.Proxy(proxy, shallow);
Decontextify.proxies.set(object, proxy2);
Decontextified.set(proxy2, object);
Expand Down Expand Up @@ -619,25 +720,46 @@ Contextify.object = (object, traps, deepTraps, flags, mock) => {
// TypeError: 'getOwnPropertyDescriptor' on proxy: trap reported non-configurability for property '<prop>'
// which is either non-existant or configurable in the proxy target

let desc;
if (!def) {
return undefined;
} else if (def.get || def.set) {
return {
desc = {
__proto__: null,
get: Contextify.value(def.get, null, deepTraps, flags) || undefined,
set: Contextify.value(def.set, null, deepTraps, flags) || undefined,
enumerable: def.enumerable === true,
configurable: def.configurable === true
};
} else {
return {
desc = {
__proto__: null,
value: Contextify.value(def.value, null, deepTraps, flags),
writable: def.writable === true,
enumerable: def.enumerable === true,
configurable: def.configurable === true
};
}
if (!desc.configurable) {
try {
def = host.Object.getOwnPropertyDescriptor(target, prop);
if (!def || def.configurable) {
local.Reflect.defineProperty(target, prop, desc);
}
} catch (e) {
// Should not happen.
}
}
return desc;
};
base.defineProperty = (target, key, descriptor) => {
let success = false;
try {
success = local.Reflect.setPrototypeOf(descriptor, null);
} catch (e) {
// Should not happen
}
if (!success) return false;
// There's a chance accessing a property throws an error so we must not access them
// in try catch to prevent contextyfing local objects.

Expand All @@ -663,10 +785,19 @@ Contextify.object = (object, traps, deepTraps, flags, mock) => {
}

try {
return host.Object.defineProperty(object, key, propertyDescriptor);
success = host.Reflect.defineProperty(object, key, propertyDescriptor);
} catch (e) {
throw Contextify.value(e);
}
if (success && descriptor.configurable) {
try {
local.Reflect.defineProperty(target, key, descriptor);
} catch (e) {
// This should not happen.
return false;
}
}
return success;
};
base.deleteProperty = (target, prop) => {
try {
Expand All @@ -689,11 +820,22 @@ Contextify.object = (object, traps, deepTraps, flags, mock) => {
}
};
base.isExtensible = target => {
let result;
try {
return Contextify.value(host.Object.isExtensible(object));
result = host.Reflect.isExtensible(object);
} catch (e) {
throw Contextify.value(e);
}
if (!result) {
try {
if (local.Reflect.isExtensible(target)) {
doPreventExtensions(target, object, obj => Decontextify.value(obj, null, deepTraps, flags));
}
} catch (e) {
// Should not happen
}
}
return result;
};
base.ownKeys = target => {
try {
Expand All @@ -703,12 +845,22 @@ Contextify.object = (object, traps, deepTraps, flags, mock) => {
}
};
base.preventExtensions = target => {
let success;
try {
host.Object.preventExtensions(object);
return true;
success = local.Reflect.preventExtensions(object);
} catch (e) {
throw Contextify.value(e);
}
if (success) {
try {
if (local.Reflect.isExtensible(target)) {
doPreventExtensions(target, object, obj => Decontextify.value(obj, null, deepTraps, flags));
}
} catch (e) {
// Should not happen
}
}
return success;
};
base.enumerate = target => {
try {
Expand All @@ -718,7 +870,7 @@ Contextify.object = (object, traps, deepTraps, flags, mock) => {
}
};

const proxy = new host.Proxy(getBaseObject(object), host.Object.assign(base, traps, deepTraps));
const proxy = new host.Proxy(createBaseObject(object), host.Object.assign(base, traps, deepTraps));
Contextify.proxies.set(object, proxy);
Contextified.set(proxy, object);
return proxy;
Expand Down
Loading

0 comments on commit d267a32

Please sign in to comment.