Skip to content

Commit

Permalink
Merge pull request #77 from caridy/caridy/issue-76
Browse files Browse the repository at this point in the history 
fixes #76: document.all == null breaks the membrane for valid targets
  • Loading branch information
@caridy
caridy authored Mar 17, 2020
2 parents 5fc9ad8 + dd82723 commit ca2349f
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 73 deletions.
75 changes: 38 additions & 37 deletions src/environment.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {
apply,
assign,
isUndefined,
isNullish,
isNullOrUndefined,
ObjectCreate,
isFunction,
hasOwnProperty,
Expand Down Expand Up @@ -31,25 +31,13 @@ import {
RawValue,
SecureValue,
RawConstructor,
SecureConstructor,
MembraneBroker,
DistortionMap,
SecureProxy,
ReverseProxy,
ReverseProxyTarget,
} from './types';

// it means it does have identity and should be proxified.
function isProxyTarget(o: RawValue | SecureValue):
o is (RawFunction | RawConstructor | RawObject | SecureFunction | SecureConstructor | SecureObject) {
// hire-wired for the common case
if (isNullish(o)) {
return false;
}
const t = typeof o;
return t === 'object' || t === 'function';
}

interface SecureEnvironmentOptions {
// Base global object used by the raw environment
rawGlobalThis: RawObject & typeof globalThis;
Expand All @@ -72,7 +60,16 @@ export class SecureEnvironment implements MembraneBroker {
throw ErrorCreate(`Missing SecureEnvironmentOptions options bag.`);
}
const { rawGlobalThis, secureGlobalThis, distortionMap } = options;
this.distortionMap = WeakMapCreate(isUndefined(distortionMap) ? [] : distortionMap.entries());
this.distortionMap = WeakMapCreate();
// validating distortion entries
distortionMap?.forEach((value, key) => {
const o = typeof key;
const d = typeof value;
if (o !== d) {
throw ErrorCreate(`Invalid distortion ${value}.`);
}
WeakMapSet(this.distortionMap, key, value);
});
// getting proxy factories ready per environment so we can produce
// the proper errors without leaking instances into a sandbox
const secureEnvFactory = secureGlobalThis.eval(`(${serializedSecureEnvSourceText})`);
Expand Down Expand Up @@ -152,10 +149,6 @@ export class SecureEnvironment implements MembraneBroker {
let currentGetter = () => undefined;
if (isFunction(originalGetter)) {
const originalOrDistortedGetter: () => any = WeakMapGet(this.distortionMap, originalGetter) || originalGetter;
if (!isProxyTarget(originalOrDistortedGetter)) {
// TODO: needs to be resilient, cannot just throw, what should we do instead?
throw ErrorCreate(`Invalid distortion.`);
}
currentGetter = function(this: any): SecureValue {
const value: RawValue = apply(originalOrDistortedGetter, env.getRawValue(this), emptyArray);
return env.getSecureValue(value);
Expand All @@ -178,39 +171,47 @@ export class SecureEnvironment implements MembraneBroker {
if (!isUndefined(secureDescriptor) &&
hasOwnProperty(secureDescriptor, 'configurable') &&
secureDescriptor.configurable === false) {
const securePropertyValue = secureValue[key];
if (isNullOrUndefined(securePropertyValue)) {
continue;
}
const t = typeof securePropertyValue;
// this is the case where the secure env has a descriptor that was supposed to be
// overrule but can't be done because it is a non-configurable. Instead we try to
// fallback to some more advanced gymnastics
if (hasOwnProperty(secureDescriptor, 'value') && isProxyTarget(secureDescriptor.value)) {
const { value: secureDescriptorValue } = secureDescriptor;
if (!WeakMapHas(this.som, secureDescriptorValue)) {
// remapping the value of the secure object graph to the outer realm graph
const { value: rawDescriptorValue } = rawDescriptor;
if (secureValue !== rawDescriptorValue) {
if (this.getRawValue(secureValue) !== rawValue) {
console.error('need remapping: ', key, rawValue, rawDescriptor);
if (hasOwnProperty(secureDescriptor, 'value')) {
// valid proxy target (intentionally ignoring the case of document.all since it is not a value descriptor)
if (t === 'function' || t === 'object') {
if (!WeakMapHas(this.som, securePropertyValue)) {
// remapping the value of the secure object graph to the outer realm graph
const { value: rawDescriptorValue } = rawDescriptor;
if (secureValue !== rawDescriptorValue) {
if (this.getRawValue(secureValue) !== rawValue) {
console.error('need remapping: ', key, rawValue, rawDescriptor);
} else {
// it was already mapped
}
} else {
// it was already mapped
// window.top is the classic example of a descriptor that leaks access to the outer
// window reference, and there is no containment for that case yet.
console.error('leaking: ', key, rawValue, rawDescriptor);
}
} else {
// window.top is the classic example of a descriptor that leaks access to the outer
// window reference, and there is no containment for that case yet.
console.error('leaking: ', key, rawValue, rawDescriptor);
// an example of this is circular window.window ref
console.info('circular: ', key, rawValue, rawDescriptor);
}
} else {
// an example of this is circular window.window ref
console.info('circular: ', key, rawValue, rawDescriptor);
}
} else if (hasOwnProperty(secureDescriptor, 'get')) {
const secureDescriptorValue = secureValue[key];
if (isProxyTarget(secureDescriptorValue)) {
if (secureDescriptorValue === secureValue[key]) {
// internationally ignoring the case of (typeof document.all === 'undefined') because
// it is specified as configurable, you never get one of those exotic objects in this branch
if (t === 'function' || t === 'object') {
if (securePropertyValue === secureValue[key]) {
// this is the case for window.document which is identity preserving getter
// const rawDescriptorValue = rawValue[key];
// this.setRefMapEntries(secureDescriptorValue, rawDescriptorValue);
// this.installDescriptors(secureDescriptorValue, rawDescriptorValue, getOwnPropertyDescriptors(rawDescriptorValue));
console.error('need remapping: ', key, rawValue, rawDescriptor);
if (ReflectIsExtensible(secureDescriptorValue)) {
if (ReflectIsExtensible(securePropertyValue)) {
// remapping proto chain
// ReflectSetPrototypeOf(secureDescriptorValue, this.getSecureValue(ReflectGetPrototypeOf(secureDescriptorValue)));
console.error('needs prototype remapping: ', key, rawValue);
Expand Down
26 changes: 12 additions & 14 deletions src/raw-value.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
ReflectGet,
ReflectSet,
map,
isNullish,
isNullOrUndefined,
unconstruct,
ownKeys,
ReflectIsExtensible,
Expand All @@ -34,7 +34,6 @@ import {
RawArray,
SecureValue,
MembraneBroker,
SecureObject,
} from './types';

function renameFunction(provider: (...args: any[]) => any, receiver: (...args: any[]) => any) {
Expand All @@ -52,16 +51,6 @@ function renameFunction(provider: (...args: any[]) => any, receiver: (...args: a
}
}

// it means it does have identity and should be proxified.
function isProxyTarget(o: RawValue): o is (SecureFunction | SecureConstructor | SecureObject) {
// hire-wired for the common case
if (isNullish(o)) {
return false;
}
const t = typeof o;
return t === 'object' || t === 'function';
}

const ProxyRevocable = Proxy.revocable;
const ProxyCreate = unconstruct(Proxy);
const { isArray: isArrayOrNotOrThrowForRevoked } = Array;
Expand Down Expand Up @@ -183,7 +172,7 @@ export function reverseProxyFactory(env: MembraneBroker) {
}
construct(shadowTarget: ReverseShadowTarget, rawArgArray: RawValue[], rawNewTarget: RawObject): RawObject {
const { target: SecCtor } = this;
if (rawNewTarget === undefined) {
if (isUndefined(rawNewTarget)) {
throw TypeError();
}
const secArgArray = env.getSecureValue(rawArgArray);
Expand Down Expand Up @@ -285,6 +274,15 @@ export function reverseProxyFactory(env: MembraneBroker) {
ReflectSetPrototypeOf(ReverseProxyHandler.prototype, null);

function getRawValue(sec: SecureValue): RawValue {
if (isNullOrUndefined(sec)) {
return sec as RawValue;
}
const t = typeof sec;
// internationally ignoring the case of (typeof document.all === 'undefined') because
// in the reserve membrane, you never get one of those exotic objects
if (t === 'function') {
return getRawFunction(sec);
}
let isSecArray = false;
try {
isSecArray = isArrayOrNotOrThrowForRevoked(sec);
Expand All @@ -293,7 +291,7 @@ export function reverseProxyFactory(env: MembraneBroker) {
}
if (isSecArray) {
return getRawArray(sec);
} else if (isProxyTarget(sec)) {
} else if (t === 'object') {
const raw = env.getRawRef(sec);
if (isUndefined(raw)) {
return createReverseProxy(sec);
Expand Down
39 changes: 20 additions & 19 deletions src/secure-value.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import {
RawConstructor,
RawFunction,
RawValue,
RawObject,
RawArray,
TargetMeta,
MembraneBroker,
Expand Down Expand Up @@ -102,16 +101,32 @@ export const serializedSecureEnvSourceText = (function secureEnvFactory(rawEnv:
return obj === undefined;
}

function isNull(obj: any): obj is null {
return obj === null;
}

function isFunction(obj: any): obj is Function {
return typeof obj === 'function';
}

function isNullish(obj: any): obj is null {
// eslint-disable-next-line eqeqeq
return obj == null;
function isNullOrUndefined(obj: any): obj is (null | undefined) {
return isNull(obj) || isUndefined(obj);
}

function getSecureValue(raw: RawValue): SecureValue {
if (isNullOrUndefined(raw)) {
return raw as SecureValue;
}
const t = typeof raw;
// NOTE: internationally checking for typeof 'undefined' for the case of
// `typeof document.all === 'undefined'`, which is an exotic object with
// a bizarre behavior described here:
// * https://tc39.es/ecma262/#sec-IsHTMLDDA-internal-slot
// This check covers that case, but doesn't affect other undefined values
// because those are covered by the previous condition anyways.
if (t === 'function' || t === 'undefined') {
return getSecureFunction(raw);
}
let isRawArray = false;
try {
isRawArray = isArrayOrNotOrThrowForRevoked(raw);
Expand All @@ -121,7 +136,7 @@ export const serializedSecureEnvSourceText = (function secureEnvFactory(rawEnv:
}
if (isRawArray) {
return getSecureArray(raw);
} else if (isProxyTarget(raw)) {
} else if (t === 'object') {
const sec: SecureValue | undefined = WeakMapGet(rom, raw);
if (isUndefined(sec)) {
return createSecureProxy(raw);
Expand Down Expand Up @@ -152,23 +167,9 @@ export const serializedSecureEnvSourceText = (function secureEnvFactory(rawEnv:
}
// if a distortion entry is found, it must be a valid proxy target
const distortedTarget = WeakMapGet(distortionMap, target) as SecureProxyTarget;
if (!isProxyTarget(distortedTarget)) {
// TODO: needs to be resilient, cannot just throw, what should we do instead?
throw ErrorCreate(`Invalid distortion.`);
}
return distortedTarget;
}

// it means it does have identity and should be proxified.
function isProxyTarget(o: RawValue | SecureValue): o is (RawFunction | RawConstructor | RawObject) {
// hire-wired for the common case
if (isNullish(o)) {
return false;
}
const t = typeof o;
return t === 'object' || t === 'function';
}

function renameFunction(rawProvider: (...args: any[]) => any, receiver: (...args: any[]) => any) {
let nameDescriptor: PropertyDescriptor | undefined;
try {
Expand Down
9 changes: 6 additions & 3 deletions src/shared.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,12 @@ export function isUndefined(obj: any): obj is undefined {
return obj === undefined;
}

export function isNullish(obj: any): obj is null {
// eslint-disable-next-line eqeqeq
return obj == null;
export function isNull(obj: any): obj is null {
return obj === null;
}

export function isNullOrUndefined(obj: any): obj is (null | undefined) {
return isNull(obj) || isUndefined(obj);
}

export function isTrue(obj: any): obj is true {
Expand Down
22 changes: 22 additions & 0 deletions test/membrane/document-all.spec.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import createSecureEnvironment from '../../lib/browser-realm.js';

describe('document.all', () => {
it('should change the value of its yellow typeof from "undefined" to "object"', function() {
// expect.assertions(2);
const evalScript = createSecureEnvironment();
expect(typeof document.all).toBe("undefined");
evalScript(`
// observable difference between a regular dom and a sandboxed dom
expect(typeof document.all).toBe("object");
`);
});
it('should work throughout the membrane', function() {
// expect.assertions(3);
const evalScript = createSecureEnvironment();
evalScript(`
expect(document.all.length > 1).toBeTrue();
expect(document.all[0].ownerDocument).toBe(document); // comparison in red
expect(document.all[0].ownerDocument === document).toBeTrue(); // comparison in yellow
`);
});
});

0 comments on commit ca2349f

Please sign in to comment.