feat(bson-parsing): restrict web worker scope COMPASS-11128

packages/shell-bson-parser/src/index.spec.ts

@@ -1,9 +1,24 @@
 import { expect } from 'chai';
+import sinon from 'sinon';
+import vm from 'vm';
+import fs from 'fs/promises';
+import path from 'path';
+import { fileURLToPath } from 'url';
+import * as WebWorkerModule from 'web-worker';
 
 import * as api from './index.js';
 import { terminateWorker } from './worker-client.js';
+import {
+  restrictGlobalScope,
+  restrictObjectPrototype,
+  ALLOWED_GLOBALS,
+  DISALLOWED_PROTOTYPE_PROPS,
+} from './worker.js';
 import { PARSE_TEST_CASES } from './../test/parse-test-cases.js';
 
+const WebWorker = (WebWorkerModule as unknown as { default: typeof Worker })
+  .default;
+
 describe('shell-bson-parser with webworker processing', function () {
   const initialWorkerScriptUrl = process.env.TEST_WORKER_SCRIPT_URL;
 
@@ -39,15 +54,156 @@ describe('shell-bson-parser with webworker processing', function () {
     }
   });
 
+  describe('restrictGlobalScope', function () {
+    it('strips capabilities not needed for parsing, keeping the JS intrinsics parsing needs', function () {
+      const scope: Record<string, unknown> = Object.create(null);
+      scope.fetch = function fetch() {};
+      scope.require = function require() {};
+      scope.process = Object.create(null);
+      scope.importScripts = function importScripts() {};
+      scope.XMLHttpRequest = function XMLHttpRequest() {};
+      scope.Object = Object;
+      scope.Array = Array;
+      scope.Math = Math;
+
+      restrictGlobalScope(scope);
+
+      expect(scope).to.not.have.property('fetch');
+      expect(scope).to.not.have.property('require');
+      expect(scope).to.not.have.property('importScripts');
+      expect(scope).to.not.have.property('XMLHttpRequest');
+      expect(scope).to.not.have.property('process');
+      expect(scope).to.have.property('Object', Object);
+      expect(scope).to.have.property('Array', Array);
+      expect(scope).to.have.property('Math', Math);
+    });
+
+    it('keeps every allowlisted global untouched', function () {
+      const scope: Record<string, unknown> = Object.create(null);
+      for (const key of ALLOWED_GLOBALS) {
+        scope[key] = key;
+      }
+
+      restrictGlobalScope(scope);
+
+      for (const key of ALLOWED_GLOBALS) {
+        expect(scope).to.have.property(key, key);
+      }
+    });
+
+    it('does not throw on non-configurable properties', function () {
+      const scope: Record<string, unknown> = Object.create(null);
+      Object.defineProperty(scope, 'nonConfigurable', {
+        value: 'danger',
+        configurable: false,
+        enumerable: true,
+      });
+
+      expect(() => restrictGlobalScope(scope)).to.not.throw();
+      expect(scope).to.have.property('nonConfigurable', 'danger');
+    });
+  });
+
+  describe('lockdownObjectPrototype', function () {
+    let originalDescriptors: Record<string, PropertyDescriptor | undefined>;
+    beforeEach(function () {
+      originalDescriptors = Object.create(null);
+      for (const key of DISALLOWED_PROTOTYPE_PROPS) {
+        originalDescriptors[key] = Object.getOwnPropertyDescriptor(
+          Object.prototype,
+          key,
+        );
+      }
+    });
+
+    afterEach(function () {
+      for (const key of DISALLOWED_PROTOTYPE_PROPS) {
+        const descriptor = originalDescriptors[key];
+        if (descriptor) {
+          Object.defineProperty(Object.prototype, key, descriptor);
+        }
+      }
+    });
+
+    it('removes every disallowed accessor from Object.prototype', function () {
+      restrictObjectPrototype();
+
+      for (const key of DISALLOWED_PROTOTYPE_PROPS) {
+        expect(Object.prototype).to.not.have.property(key);
+        expect(({} as Record<string, unknown>)[key]).to.equal(undefined);
+      }
+    });
+  });
+
   describe('terminateWorker', function () {
-    it('starts a new worker after termination', async function () {
+    let terminateSpy: sinon.SinonSpy;
+
+    beforeEach(function () {
+      terminateSpy = sinon.spy(WebWorker.prototype, 'terminate');
+    });
+
+    afterEach(function () {
+      terminateSpy.restore();
+      terminateWorker();
+    });
+
+    it('actually calls terminate() on the underlying worker, then spins up a new one', async function () {
       const res1 = await api.parse('{code: "BER"}');
       expect(res1).to.deep.equal({ code: 'BER' });
+      expect(terminateSpy.called).to.equal(false);
 
       terminateWorker();
+      expect(terminateSpy.calledOnce).to.equal(true);
 
       const res2 = await api.parse('{city: "berlin"}');
       expect(res2).to.deep.equal({ city: 'berlin' });
+
+      expect(terminateSpy.calledOnce).to.equal(true);
     });
   });
+
+  it('strips dangerous globals and locks down Object.prototype when the real worker starts', async function () {
+    const workerBundlePath = path.join(
+      path.dirname(fileURLToPath(import.meta.url)),
+      '..',
+      'dist',
+      'worker.js',
+    );
+    const code = await fs.readFile(workerBundlePath, 'utf8');
+
+    const sandbox: Record<string, unknown> = Object.create(null);
+    sandbox.postMessage = function postMessage() {};
+    sandbox.fetch = function fetch() {};
+    sandbox.require = function require() {};
+    sandbox.importScripts = function importScripts() {};
+    sandbox.XMLHttpRequest = function XMLHttpRequest() {};
+    sandbox.self = sandbox;
+    sandbox.global = sandbox;
+    sandbox.globalThis = sandbox;
+
+    vm.createContext(sandbox);
+
+    expect(sandbox).to.have.property('fetch');
+    expect(sandbox).to.have.property('require');
+    expect(sandbox).to.have.property('importScripts');
+    expect(sandbox).to.have.property('XMLHttpRequest');
+
+    vm.runInContext(code, sandbox, { filename: 'worker.js' });
+
+    expect(sandbox).to.not.have.property('fetch');
+    expect(sandbox).to.not.have.property('require');
+    expect(sandbox).to.not.have.property('importScripts');
+    expect(sandbox).to.not.have.property('XMLHttpRequest');
+
+    expect(typeof sandbox.onmessage).to.equal('function');
+
+    const stillHasProtoAccessor = vm.runInContext(
+      `Object.prototype.hasOwnProperty('__proto__')`,
+      sandbox,
+    );
+    expect(stillHasProtoAccessor).to.equal(false);
+
+    // It should not modify the default object proto
+    expect(Object.prototype).to.have.property('__proto__');
+  });
 });

packages/shell-bson-parser/src/scope.ts

@@ -73,7 +73,7 @@ const SCOPE_ANY: { [x: string]: Function } = lookupMap({
       String.prototype.substring.call(lsb, 0, 2);
     hex = msb + lsb;
 
-    const hexBuffer = Buffer.from(hex, 'hex');
+    const hexBuffer = bson.ByteUtils.fromHex(hex);
     return new bson.Binary(hexBuffer, 3);
   },
   LegacyCSharpUUID: function (u: any) {
@@ -97,7 +97,7 @@ const SCOPE_ANY: { [x: string]: Function } = lookupMap({
     const d = String.prototype.substring.call(hex, 16, 32);
     hex = a + b + c + d;
 
-    const hexBuffer = Buffer.from(hex, 'hex');
+    const hexBuffer = bson.ByteUtils.fromHex(hex);
     return new bson.Binary(hexBuffer, 3);
   },
   LegacyPythonUUID: function (u: any) {
@@ -106,21 +106,20 @@ const SCOPE_ANY: { [x: string]: Function } = lookupMap({
     }
 
     return new bson.Binary(
-      Buffer.from(
+      bson.ByteUtils.fromHex(
         String.prototype.replace.call(u, /[{}-]/g, () => ''),
-        'hex',
       ),
       3,
     );
   },
   BinData: function (t: any, d: any) {
-    return new bson.Binary(Buffer.from(d, 'base64'), t);
+    return new bson.Binary(bson.ByteUtils.fromBase64(d), t);
   },
   UUID: function (u: any) {
     if (u === undefined) {
       return new bson.UUID().toBinary();
     }
-    return new bson.Binary(Buffer.from(u.replace(/-/g, ''), 'hex'), 4);
+    return new bson.Binary(bson.ByteUtils.fromHex(u.replace(/-/g, '')), 4);
   },
   Code: function (c: any, s: any) {
     return new bson.Code(c, s);

packages/shell-bson-parser/src/worker.ts

@@ -3,7 +3,72 @@ import { trackBSON, untrackBSON } from './structured-clone-bson.js';
 
 import type { WorkerRequest, WorkerResponse } from './worker-types.js';
 
-// Exported for test
+const { self, postMessage } = globalThis;
+
+// Exported for tests
+export const ALLOWED_GLOBALS = new Set([
+  // Built-ins that acorn, bson and the evaluator reach for while parsing.
+  'Object',
+  'Array',
+  'String',
+  'Number',
+  'Boolean',
+  'Symbol',
+  'Math',
+  'Date',
+  'RegExp',
+  'JSON',
+  'Map',
+  'Set',
+  'Error',
+  'SyntaxError', // thrown by acorn on invalid input
+  'ArrayBuffer', // bson's byte utils
+  'Uint8Array',
+  'isNaN', // bson
+  'parseFloat', // acorn
+  'parseInt', // bson
+
+  // bson's byte utils pick a node or browser implementation at load time.
+  'Buffer', // node
+  'atob', // browser
+
+  // bson randomBytes, used by ObjectId() and UUID() without arguments.
+  'crypto',
+]);
+
+// Exported for tests
+export function restrictGlobalScope(scope: object): void {
+  for (const key of Object.getOwnPropertyNames(scope)) {
+    if (ALLOWED_GLOBALS.has(key)) continue;
+    try {
+      delete (scope as any)[key];
+    } catch {
+      // Non-configurable in this environment
+    }
+  }
+}
+
+// Exported for tests
+export const DISALLOWED_PROTOTYPE_PROPS = [
+  '__proto__',
+  '__defineGetter__',
+  '__defineSetter__',
+  '__lookupGetter__',
+  '__lookupSetter__',
+] as const;
+
+// Exported for tests
+export function restrictObjectPrototype(): void {
+  for (const key of DISALLOWED_PROTOTYPE_PROPS) {
+    try {
+      delete (Object.prototype as any)[key];
+    } catch {
+      // Non-configurable in this environment
+    }
+  }
+}
+
+// Exported for tests
 export function handleRequest(request: WorkerRequest): WorkerResponse {
   const { id, args } = request;
   try {
@@ -16,7 +81,9 @@ export function handleRequest(request: WorkerRequest): WorkerResponse {
 }
 
 if (typeof self !== 'undefined') {
+  restrictObjectPrototype();
+  restrictGlobalScope(globalThis);
   self.onmessage = (event: MessageEvent<WorkerRequest>) => {
-    (self as unknown as Worker).postMessage(handleRequest(event.data));
+    postMessage(handleRequest(event.data));
   };
 }