From 828b828cc727df9ffa01af41da2fa7953e7d1874 Mon Sep 17 00:00:00 2001 From: Eli Mallon Date: Sat, 7 Mar 2020 22:40:28 -0800 Subject: [PATCH 1/2] add failing test case for disallowed function constructor --- lib/__tests__/cloudworker.test.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/__tests__/cloudworker.test.js b/lib/__tests__/cloudworker.test.js index 0032933..525fe74 100644 --- a/lib/__tests__/cloudworker.test.js +++ b/lib/__tests__/cloudworker.test.js @@ -165,4 +165,14 @@ describe('cloudworker', () => { expect(res.headers.get('x-colo')).toEqual('YYZ') done() }) + + test('throws on eval', async () => { + const bindings = { EvalError: EvalError } + expect(() => { new Cloudworker(`eval('true')`, { bindings }) }).toThrow(new EvalError('Code generation from strings disallowed for this context')) // eslint-disable-line no-new + }) + + test('throws on new Function()', async () => { + const bindings = { EvalError: EvalError } + expect(() => { new Cloudworker(`new Function('true')`, { bindings }) }).toThrow(new EvalError('Code generation from strings disallowed for this context')) // eslint-disable-line no-new + }) }) From 01ec47d6ff5e7e17698cadd07e0688f7fca8d493 Mon Sep 17 00:00:00 2001 From: Eli Mallon Date: Sun, 8 Mar 2020 15:01:15 -0700 Subject: [PATCH 2/2] add FunctionProxy to disallow code execution --- lib/runtime.js | 3 ++- lib/runtime/function.js | 13 +++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 lib/runtime/function.js diff --git a/lib/runtime.js b/lib/runtime.js index cfb1e0f..dbc953b 100644 --- a/lib/runtime.js +++ b/lib/runtime.js @@ -5,6 +5,7 @@ const { FetchEvent } = require('./runtime/fetch-event') const { crypto } = require('./runtime/crypto') const { TextDecoder, TextEncoder } = require('./runtime/text-encoder') const { atob, btoa } = require('./runtime/base64') +const { FunctionProxy } = require('./runtime/function') class Context { constructor (addEventListener, cacheFactory, bindings = {}) { @@ -44,7 +45,7 @@ class Context { this.EvalError = EvalError this.Float32Array = Float32Array this.Float64Array = Float64Array - this.Function = Function + this.Function = FunctionProxy this.Int8Array = Int8Array this.Int16Array = Int16Array this.Int32Array = Int32Array diff --git a/lib/runtime/function.js b/lib/runtime/function.js new file mode 100644 index 0000000..17585de --- /dev/null +++ b/lib/runtime/function.js @@ -0,0 +1,13 @@ +// We bind `Function` in from the parent for `instanceof` +// purposes, but we don't want to disallow code generation +// from strings with new Function(). So, we use this Proxy! + +// Security note: there may well be ways to hack around this. Enforcing +// this rule is about consistency with CF workers, not allowing for +// fully untrusted code. + +module.exports.FunctionProxy = new Proxy(Function, { + construct: () => { + throw new EvalError('Code generation from strings disallowed for this context') + }, +})