laverdet / isolated-vm

Secure & isolated JS environments for nodejs
ISC License
2.19k stars 154 forks source link

`MemoryLimitExceeded` not properly triggering in `isolated-vm` #499

Closed nathang0147 closed 1 month ago

nathang0147 commented 1 month ago

Description: I am experiencing an issue with isolated-vm where the memory limit set for an isolate is not being enforced correctly during memory-intensive operations. Even though the memory limit is specified, the isolate continues executing until it crashes due to an out-of-memory error, without triggering the expected MemoryLimitExceeded error when memory usage exceeds the limit.

Steps to Reproduce:

  1. Create an isolate with a specific memory limit.
  2. Run memory-intensive code that exceeds the memory limit.
  3. Despite memory usage approaching or exceeding the limit, the MemoryLimitExceeded error is not thrown, and the isolate eventually crashes due to out-of-memory.

Expected Behavior:

Actual Behavior:

Example Code:

Copy code
import ivm from "isolated-vm";

async function run() {
  const isolate = new ivm.Isolate({ memoryLimit: 128 * 1024 * 1024 });  // 128 MB memory limit
  const context = await isolate.createContext();

  const script = await isolate.compileScript(`
    let arr = [];
    while(true) {
      arr.push(new Array(10000).fill(0));  // Continuously allocate memory until memory is exhausted
    }
  `);

  try {
    await script.run(context);
  } catch (error) {
    console.log('Error occurred:', error.message);
  }
}

run();

What I've Tried:

Environment:

Personal Diagnostics: This code will parse and evaluate if I put it into a file called main.mjs and then run node main.mjs. Yes, JavaScript includes a setTimeout function. Yes, functions are a type of primitive value in JavaScript. No, objects cannot be shared between isolates.

laverdet commented 1 month ago

There are certain classes of scripts which v8 simply cannot preempt. The Array methods are particularly vulnerable to this. I reported Array.prototype.join to the team 5 years ago which is still open today. https://issues.chromium.org/issues/42212548

I would suspect Array.prototype.fill is the same

Edit: Actually that issue linked is about Array.prototype.fill. The same issue you reported here. I don't think they're going to fix it. The best option is to polyfill Array.prototype.fill with a pure-JS implementation.

nathang0147 commented 1 month ago

Thank you all for your help! Your insights on memory limits and handling isolate disposal really helped me resolve the issue in my project. I’ve successfully implemented a solution based on the suggestions here. Closing this now—thanks again!