From f742492880c6a66dba556d838c4645fa17259430 Mon Sep 17 00:00:00 2001 From: Jason S Date: Sun, 12 Oct 2025 11:47:19 -0400 Subject: [PATCH] replaced empty object with map to fix CVE-2024-57072 --- __tests__/cjs/import.test.ts | 10 ++++++ __tests__/cjs/require.test.ts | 25 +++++++++++++++ __tests__/esm/import.test.ts | 10 ++++++ src/import.ts | 4 +-- src/require.ts | 4 +-- src/utils.ts | 57 +++++++++++++++++++++++------------ 6 files changed, 86 insertions(+), 24 deletions(-) diff --git a/__tests__/cjs/import.test.ts b/__tests__/cjs/import.test.ts index 50ec680..abb9758 100644 --- a/__tests__/cjs/import.test.ts +++ b/__tests__/cjs/import.test.ts @@ -167,6 +167,16 @@ export default code } } }) + + it('should isolate prototype modifications', async () => { + const originalToString = Object.prototype.toString + + await importFromStringFn('Object.prototype.toString = () => "hacked"; export default true') + + expect(Object.prototype.toString).toBe(originalToString) + // eslint-disable-next-line @typescript-eslint/no-base-to-string + expect({}.toString()).not.toBe('hacked') + }) } describe('importFromString', () => { diff --git a/__tests__/cjs/require.test.ts b/__tests__/cjs/require.test.ts index 0beacea..cd9a569 100644 --- a/__tests__/cjs/require.test.ts +++ b/__tests__/cjs/require.test.ts @@ -123,3 +123,28 @@ it('should use absolute filename in error stack trace', () => { } } }) + +it('should isolate global variables', () => { + global.testVar = 'original' + + requireFromString('global.testVar = "modified"') + + expect(global.testVar).toBe('original') + delete global.testVar +}) + +it('should isolate prototype modifications', () => { + const originalToString = Object.prototype.toString + + requireFromString('Object.prototype.toString = () => "hacked"') + + expect(Object.prototype.toString).toBe(originalToString) + // eslint-disable-next-line @typescript-eslint/no-base-to-string + expect({}.toString()).not.toBe('hacked') +}) + +it('should have isolated Error constructor by default', () => { + const error = requireFromString('module.exports = new Error("test")') + expect(error instanceof Error).toBe(false) + expect(error.constructor.name).toBe('Error') +}) diff --git a/__tests__/esm/import.test.ts b/__tests__/esm/import.test.ts index 04ec59d..96d7bd1 100644 --- a/__tests__/esm/import.test.ts +++ b/__tests__/esm/import.test.ts @@ -174,6 +174,16 @@ export default code } } }) + + it('should isolate prototype modifications', async () => { + const originalToString = Object.prototype.toString + + await importFromString('Object.prototype.toString = () => "hacked"; export default true') + + expect(Object.prototype.toString).toBe(originalToString) + // eslint-disable-next-line @typescript-eslint/no-base-to-string + expect({}.toString()).not.toBe('hacked') + }) }) describe('importFromStringSync', () => { diff --git a/src/import.ts b/src/import.ts index 7773615..54c3ca9 100644 --- a/src/import.ts +++ b/src/import.ts @@ -88,13 +88,13 @@ Enable '--experimental-vm-modules' CLI option or replace it with dynamic 'import const moduleFilename = getModuleFilename(dirname, filename) const moduleFileURLString = ensureFileURL(moduleFilename) - const globalObject = createGlobalObject(globals, useCurrentGlobal) + const globalMap = createGlobalObject(globals, useCurrentGlobal) const contextObject = createContextObject( { __dirname: ensurePath(dirname), __filename: ensurePath(moduleFilename) }, - globalObject + globalMap ) contextObject[IMPORTS] = {} const context = createContext(contextObject) diff --git a/src/require.ts b/src/require.ts index 4eec1d1..1e69fde 100644 --- a/src/require.ts +++ b/src/require.ts @@ -35,7 +35,7 @@ export const requireFromString = ( contextModule.filename = moduleFilename contextModule.paths = mainModule?.paths ?? [] - const globalObject = createGlobalObject(globals, useCurrentGlobal) + const globalMap = createGlobalObject(globals, useCurrentGlobal) const contextObject = createContextObject( { exports: contextModule.exports, @@ -44,7 +44,7 @@ export const requireFromString = ( __filename: contextModule.filename, __dirname: contextModule.path }, - globalObject + globalMap ) runInNewContext(code, contextObject, { diff --git a/src/utils.ts b/src/utils.ts index 80892e3..fcc07bd 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -88,29 +88,46 @@ const getCurrentGlobal = (): Context => { return currentGlobal } -export const createGlobalObject = (globals: Context, useCurrentGlobal: boolean): Context => { - const globalObject = useCurrentGlobal - ? getCurrentGlobal() - : Object.defineProperty({}, Symbol.toStringTag, { - ...Object.getOwnPropertyDescriptor(__GLOBAL__, Symbol.toStringTag) - }) - forEachPropertyKey(globals, propertyKey => { - if (propertyKey in __GLOBAL__) { - Object.defineProperty(globalObject, propertyKey, { - ...Object.getOwnPropertyDescriptor(__GLOBAL__, propertyKey), - value: globals[propertyKey as keyof Context] - }) - } else { - Object.defineProperty(globalObject, propertyKey, { - ...Object.getOwnPropertyDescriptor(globals, propertyKey) - }) - } +export const createGlobalObject = ( + globals: Context, + useCurrentGlobal: boolean +): Map => { + const globalMap = new Map() + + if (useCurrentGlobal) { + const currentGlobal = getCurrentGlobal() + Object.getOwnPropertyNames(currentGlobal).forEach(key => { + globalMap.set(key, currentGlobal[key]) + }) + Object.getOwnPropertySymbols(currentGlobal).forEach(key => { + // @ts-expect-error: safe to ignore + globalMap.set(key, currentGlobal[key]) + }) + } + + // Add user globals to Map (protected from pollution) + Object.getOwnPropertyNames(globals).forEach(key => { + globalMap.set(key, globals[key]) }) - return globalObject + Object.getOwnPropertySymbols(globals).forEach(key => { + // @ts-expect-error: safe to ignore + globalMap.set(key, globals[key]) + }) + + return globalMap } -export const createContextObject = (moduleContext: Context, globalObject: Context): Context => { - const contextObject: Context = shallowMergeContext(moduleContext, globalObject) +export const createContextObject = ( + moduleContext: Context, + globalMap: Map +): Context => { + const contextObject: Context = { ...moduleContext } + + // Convert Map back to object for VM context, but only with safe values + globalMap.forEach((value, key) => { + contextObject[key as keyof Context] = value + }) + if (!('global' in contextObject)) { contextObject.global = contextObject }