Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: do not remap crypto if remapTypedArrays is false #449

Merged
merged 1 commit into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions packages/near-membrane-dom/src/browser-realm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ function createIframeVirtualEnvironment(
instrumentation,
keepAlive = true,
liveTargetCallback,
remapTypedArrays,
remapTypedArrays = true,
signSourceCallback,
// eslint-disable-next-line prefer-object-spread
} = ObjectAssign({ __proto__: null }, providedOptions) as BrowserEnvironmentOptions;
Expand All @@ -94,7 +94,8 @@ function createIframeVirtualEnvironment(
typeof globalObjectShape !== 'object' || globalObjectShape === null;
if (shouldUseDefaultGlobalOwnKeys && defaultGlobalOwnKeys === null) {
defaultGlobalOwnKeys = filterWindowKeys(
getFilteredGlobalOwnKeys(redWindow, remapTypedArrays)
getFilteredGlobalOwnKeys(redWindow, remapTypedArrays),
remapTypedArrays
);
}
let blueConnector = blueCreateHooksCallbackCache.get(blueRefs.document) as
Expand Down Expand Up @@ -145,7 +146,7 @@ function createIframeVirtualEnvironment(
blueRefs.window,
shouldUseDefaultGlobalOwnKeys
? (defaultGlobalOwnKeys as PropertyKey[])
: filterWindowKeys(getFilteredGlobalOwnKeys(globalObjectShape)),
: filterWindowKeys(getFilteredGlobalOwnKeys(globalObjectShape), remapTypedArrays),
// Chromium based browsers have a bug that nulls the result of `window`
// getters in detached iframes when the property descriptor of `window.window`
// is retrieved.
Expand All @@ -159,7 +160,7 @@ function createIframeVirtualEnvironment(
endowments,
remapTypedArrays
);
removeWindowDescriptors(filteredEndowments);
removeWindowDescriptors(filteredEndowments, remapTypedArrays);
env.remapProperties(blueRefs.window, filteredEndowments);
}
// We intentionally skip remapping Window.prototype because there is nothing
Expand Down
20 changes: 17 additions & 3 deletions packages/near-membrane-dom/src/window.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
ReflectDeleteProperty,
ReflectGetPrototypeOf,
ReflectOwnKeys,
toSafeArray,
toSafeWeakMap,
} from '@locker/near-membrane-shared';
import { IS_CHROMIUM_BROWSER, rootWindow } from '@locker/near-membrane-shared-dom';
Expand Down Expand Up @@ -62,8 +63,8 @@ export function getCachedGlobalObjectReferences(
return record;
}

export function filterWindowKeys(keys: PropertyKey[]): PropertyKey[] {
const result: PropertyKey[] = [];
export function filterWindowKeys(keys: PropertyKey[], remapTypedArrays: boolean): PropertyKey[] {
const result: PropertyKey[] = toSafeArray([]);
let resultOffset = 0;
for (let i = 0, { length } = keys; i < length; i += 1) {
const key = keys[i];
Expand All @@ -79,6 +80,12 @@ export function filterWindowKeys(keys: PropertyKey[]): PropertyKey[] {
result[resultOffset++] = key;
}
}

// Crypto and typed arrays must be from the same global object
if (remapTypedArrays === false) {
result.splice(result.indexOf('Crypto'), 1);
rwaldron marked this conversation as resolved.
Show resolved Hide resolved
}

return result;
}

Expand Down Expand Up @@ -109,14 +116,21 @@ export function filterWindowKeys(keys: PropertyKey[]): PropertyKey[] {
* that will be installed (via the membrane) as global descriptors in
* the red realm.
*/
export function removeWindowDescriptors<T extends PropertyDescriptorMap>(unsafeDescs: T): T {
export function removeWindowDescriptors<T extends PropertyDescriptorMap>(
unsafeDescs: T,
remapTypedArrays: boolean
): T {
// Remove unforgeable descriptors that cannot be installed.
ReflectDeleteProperty(unsafeDescs, 'document');
ReflectDeleteProperty(unsafeDescs, 'location');
ReflectDeleteProperty(unsafeDescs, 'top');
ReflectDeleteProperty(unsafeDescs, 'window');
// Remove other browser specific unforgeables.
ReflectDeleteProperty(unsafeDescs, 'chrome');
// Crypto and typed arrays must be from the same global object
if (remapTypedArrays === false) {
ReflectDeleteProperty(unsafeDescs, 'Crypto');
}
return unsafeDescs;
}

Expand Down
24 changes: 24 additions & 0 deletions test/membrane/binary-data.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,30 @@ describe('Crypto', () => {
endowments: Object.getOwnPropertyDescriptors({ expect }),
});

env.evaluate(`
expect(() => {
crypto.getRandomValues(new Uint8Array(1));
}).not.toThrow();
`);
});
it('works when typed arrays are not remapped', () => {
const env = createVirtualEnvironment(window, {
endowments: Object.getOwnPropertyDescriptors({ expect }),
remapTypedArrays: false,
});

env.evaluate(`
expect(() => {
crypto.getRandomValues(new Uint8Array(1));
}).not.toThrow();
`);
});
it('ignores the presense of crypto in endowments if remapTypedArrays is false', () => {
const env = createVirtualEnvironment(window, {
endowments: Object.getOwnPropertyDescriptors({ Crypto, crypto, expect }),
remapTypedArrays: false,
});

env.evaluate(`
expect(() => {
crypto.getRandomValues(new Uint8Array(1));
Expand Down
Loading