Skip to content

Commit

Permalink
Merge pull request #367 from XmiliaH/security-fix
Browse files Browse the repository at this point in the history
Security Fixes
  • Loading branch information
XmiliaH authored Oct 12, 2021
2 parents 82caa5b + b4f6e2b commit 6820f56
Show file tree
Hide file tree
Showing 4 changed files with 201 additions and 51 deletions.
131 changes: 128 additions & 3 deletions lib/contextify.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ local.Reflect.isExtensible = Reflect.isExtensible;
local.Reflect.preventExtensions = Reflect.preventExtensions;
local.Reflect.getOwnPropertyDescriptor = Reflect.getOwnPropertyDescriptor;

function curry(func) {
function uncurryThis(func) {
return (thiz, args) => local.Reflect.apply(func, thiz, args);
}

const FunctionBind = curry(Function.prototype.bind);
const FunctionBind = uncurryThis(Function.prototype.bind);

// 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 @@ -69,7 +69,7 @@ const Contextified = new host.WeakMap();
const Decontextified = new host.WeakMap();

// We can't use host's hasInstance method
const ObjectHasInstance = curry(local.Object[Symbol.hasInstance]);
const ObjectHasInstance = uncurryThis(local.Object[Symbol.hasInstance]);
function instanceOf(value, construct) {
try {
return ObjectHasInstance(construct, [value]);
Expand Down Expand Up @@ -1001,6 +1001,131 @@ BufferOverride.inspect = function inspect(recurseTimes, ctx) {
};
const LocalBuffer = global.Buffer = Contextify.readonly(host.Buffer, BufferMock);
Contextify.connect(host.Buffer.prototype.inspect, BufferOverride.inspect);
Contextify.connect(host.Function.prototype.bind, Function.prototype.bind);

const oldPrepareStackTraceDesc = Reflect.getOwnPropertyDescriptor(Error, 'prepareStackTrace');

let currentPrepareStackTrace = Error.prepareStackTrace;
const wrappedPrepareStackTrace = new host.WeakMap();
if (typeof currentPrepareStackTrace === 'function') {
wrappedPrepareStackTrace.set(currentPrepareStackTrace, currentPrepareStackTrace);
}

let OriginalCallSite;
Error.prepareStackTrace = (e, sst) => {
OriginalCallSite = sst[0].constructor;
};
new Error().stack;
if (typeof OriginalCallSite === 'function') {
Error.prepareStackTrace = undefined;

function makeCallSiteGetters(list) {
const callSiteGetters = [];
for (let i=0; i<list.length; i++) {
const name = list[i];
const func = OriginalCallSite.prototype[name];
callSiteGetters[i] = {__proto__: null,
name,
propName: '_' + name,
func: (thiz) => {
return local.Reflect.apply(func, thiz, []);
}
};
}
return callSiteGetters;
}

function applyCallSiteGetters(callSite, getters) {
const properties = {__proto__: null};
for (let i=0; i<getters.length; i++) {
const getter = getters[i];
properties[getter.propName] = {
__proto__: null,
value: getter.func(callSite)
};
}
return properties;
}

const callSiteGetters = makeCallSiteGetters([
'getTypeName',
'getFunctionName',
'getMethodName',
'getFileName',
'getLineNumber',
'getColumnNumber',
'getEvalOrigin',
'isToplevel',
'isEval',
'isNative',
'isConstructor',
'isAsync',
'isPromiseAll',
'getPromiseIndex'
]);

class CallSite {
constructor(callSite) {
Object.defineProperties(this, applyCallSiteGetters(callSite, callSiteGetters));
}
getThis() {return undefined;}
getFunction() {return undefined;}
toString() {return 'CallSite {}';}
}

(function setupCallSite() {
for (let i=0; i<callSiteGetters.length; i++) {
const name = callSiteGetters[i].name;
const propertyName = callSiteGetters[i].propName;
const func = {func() {
return this[propertyName];
}}.func;
const nameProp = Object.getOwnPropertyDescriptor(func, 'name');
nameProp.value = name;
Object.defineProperty(func, 'name', nameProp);
const funcProp = Object.getOwnPropertyDescriptor(OriginalCallSite.prototype, name);
funcProp.value = func;
Object.defineProperty(CallSite.prototype, name, funcProp);
}
})();

Object.defineProperty(Error, 'prepareStackTrace', {
configurable: false,
enumerable: false,
get() {
return currentPrepareStackTrace;
},
set(value) {
if (typeof(value) !== 'function') {
currentPrepareStackTrace = value;
return;
}
const wrapped = wrappedPrepareStackTrace.get(value);
if (wrapped) {
currentPrepareStackTrace = wrapped;
return;
}
const newWrapped = (error, sst) => {
if (host.Array.isArray(sst)) {
for (let i=0; i<sst.length; i++) {
const cs = sst[i];
if (typeof cs === 'object' && local.Reflect.getPrototypeOf(cs) === OriginalCallSite.prototype) {
sst[i] = new CallSite(cs);
}
}
}
return value(error, sst);
};
wrappedPrepareStackTrace.set(value, newWrapped);
wrappedPrepareStackTrace.set(newWrapped, newWrapped);
currentPrepareStackTrace = newWrapped;
}
});
} else if (oldPrepareStackTraceDesc) {
Reflect.defineProperty(Error, 'prepareStackTrace', oldPrepareStackTraceDesc);
} else {
Reflect.deleteProperty(Error, 'prepareStackTrace');
}


const exportsMap = host.Object.create(null);
Expand Down
87 changes: 59 additions & 28 deletions lib/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -544,17 +544,17 @@ function doWithTimeout(fn, timeout) {
}
}

/**
* Creates the hook to check for the use of async.
*
* @private
* @param {*} internal - The internal vm object.
* @return {*} The hook function
*/
function makeCheckAsync(internal) {
function tryCompile(args) {
const code = args[args.length - 1];
const params = args.slice(0, -1);
vm.compileFunction(code, params);
}

function makeCheckHook(checkAsync, checkImport) {
if (!checkAsync && !checkImport) return null;
return (hook, args) => {
if (hook === 'function' || hook === 'generator_function' || hook === 'eval' || hook === 'run') {
const funcConstructor = internal.Function;
if (hook === 'function' || hook === 'generator_function' || hook === 'eval' || hook === 'run' ||
(!checkAsync && (hook === 'async_function' || hook === 'async_generator_function'))) {
if (hook === 'eval') {
const script = args[0];
args = [script];
Expand All @@ -563,28 +563,41 @@ function makeCheckAsync(internal) {
// Next line throws on Symbol, this is the same behavior as function constructor calls
args = args.map(arg => `${arg}`);
}
if (args.findIndex(arg => /\basync\b/.test(arg)) === -1) return args;
const asyncMapped = args.map(arg => arg.replace(/async/g, 'a\\u0073ync'));
const hasAsync = checkAsync && args.findIndex(arg => /\basync\b/.test(arg)) !== -1;
const hasImport = checkImport && args.findIndex(arg => /\bimport\b/.test(arg)) !== -1;
if (!hasAsync && !hasImport) return args;
const mapped = args.map(arg => {
if (hasAsync) arg = arg.replace(/async/g, 'a\\u0073ync');
if (hasImport) arg = arg.replace(/import/g, 'i\\u006dport');
return arg;
});
try {
// Note: funcConstructor is a Sandbox object, however, asyncMapped are only strings.
funcConstructor(...asyncMapped);
tryCompile(mapped);
} catch (u) {
// u is a sandbox object
// Some random syntax error or error because of async.
// Some random syntax error or error because of async or import.

// First report real syntax errors
try {
// Note: funcConstructor is a Sandbox object, however, args are only strings.
funcConstructor(...args);
} catch (e) {
throw internal.Decontextify.value(e);
tryCompile(args);

if (hasAsync && hasImport) {
const mapped2 = args.map(arg => arg.replace(/async/g, 'a\\u0073ync'));
try {
tryCompile(mapped2);
} catch (e) {
throw new VMError('Async not available');
}
throw new VMError('Dynamic Import not supported');
}
if (hasAsync) {
// Then async error
throw new VMError('Async not available');
}
// Then async error
throw new VMError('Async not available');
throw new VMError('Dynamic Import not supported');
}
return args;
}
throw new VMError('Async not available');
if (checkAsync) throw new VMError('Async not available');
return args;
};
}

Expand Down Expand Up @@ -708,7 +721,7 @@ class VM extends EventEmitter {
// Create the bridge between the host and the sandbox.
const _internal = CACHE.contextifyScript.runInContext(_context, DEFAULT_RUN_OPTIONS).call(_context, require, HOST);

const hook = fixAsync ? makeCheckAsync(_internal) : null;
const hook = makeCheckHook(fixAsync, true);

// Define the properties of this object.
// Use Object.defineProperties here to be able to
Expand Down Expand Up @@ -1172,7 +1185,22 @@ class NodeVM extends VM {
let script;

if (code instanceof VMScript) {
script = this.options.strict ? code._compileNodeVMStrict() : code._compileNodeVM();
if (this._hook) {
const prefix = this.options.strict ? STRICT_MODULE_PREFIX : MODULE_PREFIX;
const scriptCode = prefix + code.getCompiledCode() + MODULE_SUFFIX;
const changed = this._hook('run', [scriptCode])[0];
if (changed === scriptCode) {
script = this.options.strict ? code._compileNodeVMStrict() : code._compileNodeVM();
} else {
script = new vm.Script(changed, {
filename: code.filename,
displayErrors: false,
importModuleDynamically
});
}
} else {
script = this.options.strict ? code._compileNodeVMStrict() : code._compileNodeVM();
}
resolvedFilename = pa.resolve(code.filename);
dirname = pa.dirname(resolvedFilename);
} else {
Expand All @@ -1185,8 +1213,11 @@ class NodeVM extends VM {
dirname = null;
}
const prefix = this.options.strict ? STRICT_MODULE_PREFIX : MODULE_PREFIX;
script = new vm.Script(prefix +
this._compiler(code, unresolvedFilename) + MODULE_SUFFIX, {
let scriptCode = prefix + this._compiler(code, unresolvedFilename) + MODULE_SUFFIX;
if (this._hook) {
scriptCode = this._hook('run', [scriptCode])[0];
}
script = new vm.Script(scriptCode, {
filename: unresolvedFilename,
displayErrors: false,
importModuleDynamically
Expand Down
4 changes: 3 additions & 1 deletion lib/sandbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,10 @@ return ((vm, host) => {

const code = host.STRICT_MODULE_PREFIX + contents + host.MODULE_SUFFIX;

const ccode = vm._hook('run', [code]);

// Precompile script
script = new Script(code, {
script = new Script(ccode, {
__proto__: null,
filename: filename || 'vm.js',
displayErrors: false,
Expand Down
30 changes: 11 additions & 19 deletions test/vm.js
Original file line number Diff line number Diff line change
Expand Up @@ -910,31 +910,23 @@ describe('VM', () => {
});

if (NODE_VERSION >= 10) {
it('Dynamic import attack', (done) => {
process.once('unhandledRejection', (reason) => {
assert.strictEqual(reason.message, 'process is not defined');
done();
});
it('Dynamic import attack', () => {

const vm2 = new VM();

vm2.run(`
(async () => {
try {
await import('oops!');
} catch (ex) {
// ex is an instance of NodeError which is not proxied;
const process = ex.constructor.constructor('return process')();
const require = process.mainModule.require;
const child_process = require('child_process');
const output = child_process.execSync('id');
process.stdout.write(output);
}
})();
`);
assert.throws(()=>vm2.run(`
const process = import('oops!').constructor.constructor('return process')();
`), /VMError: Dynamic Import not supported/);
});
}

it('Error.prepareStackTrace attack', () => {
const vm2 = new VM();
const sst = vm2.run('Error.prepareStackTrace = (e,sst)=>sst;const sst = new Error().stack;Error.prepareStackTrace = undefined;sst');
assert.strictEqual(vm2.run('sst=>Object.getPrototypeOf(sst)')(sst), vm2.run('Array.prototype'));
assert.throws(()=>vm2.run('sst=>sst[0].getThis().constructor.constructor')(sst), /TypeError: Cannot read property 'constructor' of undefined/);
});

after(() => {
vm = null;
});
Expand Down

0 comments on commit 6820f56

Please sign in to comment.