"Sandbox" break-out via dlopen

Howdy,

I've identified an interesting attack scenario against Jade/Pug. Not a vulnerability per se, but a bypass of the restricted namespace Pug implements. Take a gander at my full write-up: http://blog.blindspotsecurity.com/2016/09/nodejs-breaking-jade-pug-dlopen.html

Not a huge issue, but you might consider finding a way to block access to process.dlopen, since it is unlikely anyone needs it.

Thanks, tim

Pug implements no sandbox of any sort, nor "restricted namespace". The fact that module, exports, and require are not available is merely a result of the way Node.js implements them (they are not globals).
Because we have no sandbox, a lot more damage that can be done if an untrusted template is executed on the server. One of my favorite is (assuming the unfortunate soul passed fs to the template:
```
-
 function haha(module) {
   try { fs.writeFileSync(module.filename, 'console.log("uve been h4ck3d")'); } catch (err) {}
   for (var mod of module.children) {
     haha(mod);
   }
 }
 haha(process.mainModule);
 process.exit();
```
But other simpler exploits are possible too:
```
- var i = 0
- while (true) if (++i !== process.pid) try { process.kill(i) } catch (err) {}
```
```
- while (true) {}
```
The point is, developers should NEVER execute untrusted code on their server, even with Pug. Simply blocking one function will never make the server secure.
This "attack" is not specific to Pug. All other template engines using new Function() or worse, eval() have this problem. That includes Handlebars, Lodash, doT, Nunjucks, EJS, and pretty much every single other template engines I can think of.
Despite the fact that we don't support running user's code on the server, I am not opposed to the idea of using a module to mitigate this, but it would be out of scope to implement a sandbox in Pug itself.

I totally agree with you that developers should never trust users to supply server-side Pug code. However, given how little is available from within Pug' namespace (unless developers explicitly expose things) naive developers might be tempted to think there is some kind of built-in protection against eval() injections or what have you.

Your examples in (2) above are of course things I already thought about. But fs isn't available by default. And if I'm trying to hack your server, I really don't care about process.kill. I mean, yeah it's a DoS, but if I already have code injection (however limited) I'm not going to waste my vulnerability on just crashing some processes. So process.dlopen is a bit unexpected, given its poorly documented nature.

This little blog post is hopefully meant to help drive home that these restrictive namespaces shouldn't be the first line of defense, as you say, in this context in other templating languages. I'm not trying to pick on Pug, just trying to raise awareness. But also, why not block access to process.dlopen, or even all of process by default? Are there documented use cases where developers expect that to be available by default?

Most developers aren't experts in Pug or any other templating language. And you can't expect them to be. Why not block the things that they never need, especially if those things carry any security risk at all? I'm not asking for a full sandbox or any published security guarantees, and I'm not saying it's your responsibility to prevent developers from doing something stupid, but if we're going to change the disaster that is infosec we need to be a bit more defensive at a basic level.

@ecbftw If there was a real security issue here (fortunately there isn't) this would be a very irresponsible disclosure. In the future, I would suggest https://nodesecurity.io/report as a good place to disclose security issues. They will then take care of contacting the package's maintainers before you publish a blog post about it.

Having read your article, I think the tone of it is largely deceptive and unhelpful. The core message that comes across is "Pug attempted to build their own sandbox, which is very difficult, and didn't do it properly." Instead, the message should be, "To run un-trusted code, you need to use a proper sandbox, e.g. docker or a VM". By characterising your observation that pug is not a sandbox as a security vulnerability, you spread more fear, uncertainty and doubt. This makes it much harder to give real security vulnerabilities the attention they deserve. If we were to mark past versions of pug as insecure, after "fixing" this issue, we would be implying that all dependent projects needed to urgently update, which isn't the case.

To address the actual concern itself, I think the key thing to be aware of here is that we can't make pug a true sandbox because the overhead would far exceed tolerable levels. pug doesn't actively hide any globals, it's simply that require is not a global. We never pretend that pug is a sandbox.

It would be irresponsible to actively hide process.dlopen when you can just as easily break out using Object.__proto__ which would allow you to modify the behaviour of almost any part of most applications. Pretending we have a true sandbox by hiding a few globals is not an option. As for hiding the whole process object, it's fairly likely that some templates in the wild make use of process.version. Also did you know you can read un-zeroed memory via new Buffer(99999999)?

What we can (and probably should) do, is add clearer documentation to the website to warn people that they should not run un-trusted pug templates without a proper sandbox.

Hi @ForbesLindesay. Thank you for taking the time to read my blog post.

For now I'm going to avoid responding to your accusations and misinterpretations of what I'm trying to do. I'd like to keep the discussion productive at a technical level and I'm happy to write another blog post to correct anything I misrepresented, once I fully understand some alternative ways to execute arbitrary code.

Thank you for the tip on using Object.__proto__. My guess here is that you're proposing one could override Object.__proto__ to trigger your own code to run shortly after Pug's render() method completes. In that way, the code would have access to the namespace/scope of the calling code. Is that correct?

I haven't been able to get it working, (based on the __proto__ documentation from Mozilla). As a first step, clearly I should be able to override another object's prototype within my own Pug scope first, and if that works, I can try to attack objects/methods that exist outside of the template. I've tried stuff like this:

- var proto = { a: function () { console.log('a'); }}
- Object.__proto__ = proto; // one way
- Object.prototype = proto; // another way
- Object.setPrototypeOf(Object, proto); // yet another way.  love JavaScript
- var x = new Object(); // also tried "var x = {};"
- x.a(); // blows up, method not defined

Any idea what am I doing wrong? Thank you.

I'm not going to give a tutorial on breaking out of JavaScript VMs here. It's a fairly well covered field. By adding setters and getters to objects you can intercept things like JSON data being sent by the server to other users. This is not what's important here though as that's well established. If you want to correct your mistakes, this is not what you need to be focusing on.

For now I'm going to avoid responding to your accusations and misinterpretations of what I'm trying to do.

I'm doing my best to assume good intent here. I assume you thought you had uncovered a security vulnerability that you should report, and didn't realise that a public blog post and GitHub issue is the wrong way to report security vulnerabilities.

Unfortunately, whether or not your intentions were good is not really the point here. Anyone reading that blog post comes away with the impression they've just read about a security flaw in pug. This impression is your responsibility to fix. As for correcting things in a follow up post: which do you think will generate more readership:

OMG look at all the security issues I found in pug!
Sorry, turns I hadn't found any security issues after all.

The first of these articles might continue showing up in google search results for years, with almost nobody reading the second article. If you make a mistake, own up to it. Publish a big correction inline in the article along the lines of:

CORRECTION: When I initially wrote this article, I thought I had found an exploitable security flaw in pug. After discussions with @ForbesLindesay && @TimothyGu I now understand that this does not represent a genuine security issue. You should, however, never run un-trusted code in any programming language, without a proper sandbox in place. This article still demonstrates that pug is not a sandbox.

If there's one thing you take away from this though, it's that a public blog post/GitHub issue is never the correct place to disclose a security vulnerability.

As much as I try to cool tempers (my own and yours) you seem to be determined to inflame the situation, so I won't waste any more time on this thread unless I see a dramatic change in your tone.

I assume you thought you had uncovered a security vulnerability that you should report, and didn't realise that a public blog post and GitHub issue is the wrong way to report security vulnerabilities.

You know what they say about "assume", right? Well you're wrong, I never saw this as a security vulnerability in Pug and I don't feel it came across that way in the blog post. Security techniques are still useful in my community even if they aren't CVE worthy. Pentesters regularly perform security assessments on application development platforms they are not experts in. We simply can't be experts on every single platform, since kids these days invent a new platform every week. This is why it is useful to have a step-by-step way to prove to a customer that their app is very insecure if Pug code injection is possible in their app's templates.

I've been doing responsible disclosure of security vulnerabilities since 2002. I would be surprised if many of the people on Pug's team have been professional developers that long, let alone be as familiar with security community norms as I am. So please don't get up on your high horse about what you consider responsible disclosure. "Responsible disclosure" means different things to different people. If I had a "real" vulnerability, then as a researcher I do not owe you, the software developer, anything whatsoever. Researchers put significant time and effort into understanding the security ramifications of bugs, misconfigurations, and other failures which developers like you rarely appreciate. You may prefer to be provided this information in certain ways, but if you receive security information (vulnerabilities or otherwise), you should see these as a contribution to your project, regardless of how it came to you. I didn't have to notify you at all. I didn't have to spend a lot of time explaining myself while under verbal attack. Work with researchers, rather than being adversarial. It will serve you in the long run.

If this were a blatant fault in Pug's code, I would have used different channels, since I happen to prefer responsible disclosure. Since I'm a fairly new to the Node.js platform, I even checked with a renowned expert in Node security before publicly posting this. He agreed that it wasn't something that needed to be disclosed privately. So I did my homework on this to be sure I wasn't harming the community. Have you done yours?

When I see you and @TimothyGu throwing out a variety of hypothetical ways to break out of the restricted namespace, but then these don't pan out or aren't arbitrary code execution, I get suspicious. When you don't even link me to any content that backs up your claims, I'm doubtful you have all of the facts straight. In my line of work, it isn't enough to hand wave about what might be possible. We have to demonstrate the exploit. As much as we scream to developers that they shouldn't do X because of a security risk, they rarely believe us until we demonstrate popping a shell on their box. So if you're going to say "@ecbftw, your shit doesn't matter because (A) and (B) let you do this anyway", then at least point me to something that has concrete info on (A) and (B). I don't expect you to write me an exploit, since you're not security experts. I'll happily test it, but at least get me started, since you are the JavaScript experts.

Unfortunately, whether or not your intentions were good is not really the point here. Anyone reading that blog post comes away with the impression they've just read about a security flaw in pug.

I strongly disagree. Did you read my entire post, or just skim it? Please see:

Finally, I just want to make clear that I don't really fault the Pug developers for allowing this to occur. The code execution restrictions they have implemented really should be seen as a best-effort security measure.

And in my original issue posting:

Not a vulnerability per se, but a bypass of the restricted namespace Pug implements.

Yeah, I had some misconceptions here about what bit of code was restricting the namespace (Pug vs whatever), but regardless, I made it clear that Pug wasn't at fault for anything.

Since you're being decidedly unhelpful, I'll post some sort of update clarifying my understanding of things when I have time to do all of the research. It might be a while though, since I have a full plate.

pugjs / pug

"Sandbox" break-out via dlopen #2521