Node v0.10.21 Stable has critical security fix(blog.nodejs.org)

132 pointsby aaronblohowiakhá 13 anos49 comments

blog.nodejs.org

Node v0.10.21 Stable has critical security fix

http://blog.nodejs.org/2013/10/18/node-v0-10-21-stable/

15 comments

meritthá 13 anos

HTTP Pipelining is designed so a client can send multiple requests without waiting on a response and then the server sends all the responses in order. This is helpful on high latency links since it can combine numerous HTTP requests into fewer packets. The exploit is the client never stops to read and just writes requests nonstop. Meanwhile node's http.Server continually populates a response buffer which is never consumed.

Node uses Stream[1] objects for reading/writing streams of data. The Stream object has a 'needsDrain' boolean which is set once its internal buffer surpasses the highWaterMark (defaults to 16kb). Subsequent writes will return false[2] and code should wait until the 'drain' event is emitted, signaling it's safe to write again[3]. The documentation even warns about this scenario:

> However, writes will be buffered in memory, so it is best not to do this excessively. Instead, wait for the drain event before writing more data.

http.Server[4] uses a writeable stream to send responses to a client. Until this patch[5] it was ignoring the needsDrain/highWaterMark status and just writing to the stream. It fills up the buffer of the writeable stream, far beyond the high water mark and eventually runs out of memory.

The patch resolves this by checking when needsDrain is set, then it stops writing and stops reading/parsing incoming data. It then waits until the 'drain' event is fired and then proceeds as normal.

[1] http://nodejs.org/api/stream.html

[2] http://nodejs.org/api/stream.html#stream_writable_write_chun...

[3] http://nodejs.org/api/stream.html#stream_event_drain

[4] http://nodejs.org/api/http.html#http_class_http_server

[5] https://github.com/joyent/node/commit/085dd30e93da67362f044a...

xs_kidhá 13 anos

- No CVE.

- Distributions weren't contacted prior the release.

- Everyone can see the diff for the fix in the codebase.

- There are a PoC as test-case in the code.

- The release was done in the start of weekend when everyone in America is leaving the office and everyone in Europe is sleeping.

- A big part of the community is in two conferences right now.

IMHO that was the worst way to provide a security update.

glenjaminhá 13 anos

The initial report was made publicly, including a proof of concept exploit, and is currently viewable via google cache. The censored issue report is referenced with a metasploit pull request containing code that can be used to exploit.

Given this, I'd say sooner is better than later.

It would be prudent to mention making your load balancer limit the number of requests than can be pipelined down a single connection should resolve any issue.

xs_kidhá 13 anos

The metasploit exploit was possible only thanks to the release (see the tweets of metasploit maintainer @hdmoore), IMO they could have waited until Monday morning to release it.

ehsanu1há 13 anos

While the security update protocol leaves something to be desired, the possibility of a DoS attach is not quite as bad as a vulnerability that leaks real information. If that were to happen, I'd hope all the steps you've outlined would be followed.

tux1968há 13 anos

Agreed, however it would be nice to test and reinforce those steps at a time like this when they're not as crucially important.

microbhá 13 anos

What conferences?

xs_kidhá 13 anos

JSConfCo: http://jsconf.co/

RealtimeConf: http://2013.realtimeconf.com/

rmrfrmrfhá 13 anos

Boohoo; 1 or 2 Hubots and Ghost blogs might crash over the weekend. If you're as paranoid as you seem to be, you have someone on call to fix this and wouldn't have dared to have put such an untested technology out in the wild.

jared314há 13 anos

Apparently, a trivial DoS vulnerability for any Node serving HTTP:

https://groups.google.com/forum/#!msg/nodejs/NEbweYB0ei0/gWv...

The odd thing about non-disclosure in an open source project is: I can diff the code bases before and after the fix.

https://github.com/joyent/node/issues/6214

https://github.com/joyent/node/commit/085dd30e93da67362f044a...

And, they have a test script:

https://github.com/joyent/node/blob/085dd30e93da67362f044ad1...

jared314há 13 anos

And, now metasploit has two competing modules in the works:

https://github.com/rapid7/metasploit-framework/pull/2548

https://github.com/rapid7/metasploit-framework/pull/2549

jphhá 13 anos

Node team: you're censoring the original ticket, which is unwise IHMO.

Your approach makes it impossible for an honest sysadmin to quickly find a way to block the attack using a firewall, but your approach doesn't stop an attacker from building an exploit based on the public commit.

Someone will come up with a proof of concept exploit quickly, and post it, probably here.

Please do the right thing: un-censor the GitHub ticket so we can understand what's happening.

benatkinhá 13 anos

I'm sure they're well aware of this argument.

> Your approach makes it impossible for an honest sysadmin to quickly find a way to block the attack using a firewall, but your approach doesn't stop an attacker from building an exploit based on the public commit.

This is unfair. You're implying that sysadmins don't have access to programming resources, but that attackers do, without actually coming out and saying it.

Once it's expressed this way, it seems wrongheaded. The phrase "script kiddies" comes out of attackers doing a lot without knowing much about programming. There are many sysadmins who code, and many attackers who don't. Furthermore, I think attackers are more likely to act alone than sysadmins, who often have developers working with them whom they can ask to help.

Finally, as far as I can tell this is self-censorship. The people who created the ticket participated in the decision to hide it, or aren't loudly objecting to it. This type of "censorship" is not to be confused with more serious forms of censorship.

xs_kidhá 13 anos

PoC is in codebase, it was published as a test-case for the fix.

jkremshá 13 anos

Unfortunately that test-case passes against 0.8.25 as well. So I'm not quite convinced it can be used reliably to reproduce the problem.

ak217há 13 anos

Unsurprising because the new streams API, which is responsible for this bug, was introduced in node 0.10. Try with earlier 0.10 versions instead.

jkremshá 13 anos

Well, in that case lets add confusion about affected versions to the list of things being suboptimal about this whole thing: http://blog.nodejs.org/2013/10/18/node-v0-8-26-maintenance/ - the same warning about the same error and they did backport the fix to 0.8: https://github.com/joyent/node/commit/653d4db71f569ddc87a0bc...

ak217há 13 anos

You're right, that is confusing. I'm not sure how it's exploitable in 0.8.

dysingerhá 13 anos

https://github.com/dysinger/node-schadenfreude

viraptorhá 13 anos

So what they did is: release a new version with security and other changes mixed, on Friday, and didn't provide any details so you can't check if you're vulnerable post-upgrade. They could really handle it a bit better.

revelationhá 13 anos

They did provide plenty detail, just not to the guy on call who has to read the announcement and react quickly.

Meanwhile, they have the changelog and a test PoC for the one looking to exploit it.

dcuhá 13 anos

Making public a critical vulnerability on friday night is not a nice movement.

EDIT: Oh and there's an exploit already.

xs_kidhá 13 anos

sure, they published a PoC in the code: https://github.com/joyent/node/blob/085dd30e93da67362f044ad1...

rdtschá 13 anos

> https://github.com/joyent/node/commit/085dd30e93da67362f044a...

Looks like a flood of concurrent requests will just fill up the memory

jgreen10há 13 anos

I was considering using Node.js for a new project, but I quickly backed away from it when I realized it didn't do basic flow control properly.

AndyKelleyhá 13 anos

So you started considering Node.js, and immediately started reading the http parsing code?

...joking aside, I'm curious what you saw that made you realize basic flow control was broken?

sillysaurus2há 13 anos

So you started considering Node.js, and immediately started reading the http parsing code?

Well, there's no way to know whether something is reliable for your purposes unless you understand how it works... Is it uncommon to read code?

AndyKelleyhá 13 anos

It struck me as an oddly specific part of a large codebase to immediately jump to, but in retrospect it makes sense.

I'm all for reading code, but I think that if you can get away with not reading code, it's actually a good sign - it means all the abstractions are holding up. jgreen10 probably didn't go off and start reading the assembly code that is generated when you compile Node.js. Again, I agree with your larger point. I just want to be careful before snubbing my nose to those who don't always read up on modules they use before using them.

mh-há 13 anos

Is it uncommon to read code?

I didn't think so, but it's starting to seem that way.

rdtschá 13 anos

Funny I noticed this on an internal server as well but chucked to an older version. Hoping it "clearly is fixed in latest code, something so glaringly obviously broken, wouldn't be hanging around too long with all the hype surrounding node these days..."

Anyway, I wouldn't stick node out exposed to the outside world. Granted sticking nginx in front presumely won't help with this issue. Just keep feeding a 4GB file to it and it will crash the back-end [EDIT: n.m. I am not sure anymore, someone mentions it is possible to mitigate it that way]

Yikes, this is a bad one. Glad they fixed it. But it leaves me with the same impression I had after finding out how MongoDB used to have unacknowledged writes turned on by default, and people's data was silently getting corrupted.

sillysaurus2há 13 anos

Granted sticking nginx in front presumely won't help with this issue. Just keep feeding a 4GB file to it and it will crash the back-end.

Why does that happen? nginx can't help here?

rdtschá 13 anos

See mathrawka's reply (I haven't test it though)

BTW, I just ran memory of my server into swap with this:

    $ dd if=/dev/zero of=2g bs=1M count=2048

    $ curl -F "2g=@2g" <myresource>

(EDIT: explanation, this creates 2g file then uploads it <myresource> as a file upload -- multipart mime. @ sign just insert the named file data into the form)

mathrawkahá 13 anos

And that is why you should never blindly use bodyParser middleware in production...

http://andrewkelley.me/post/do-not-use-bodyparser-with-expre...

Wilyahá 13 anos

Wouldn't client_max_body_size (which is set to 1MB by default) in nginx config prevent the 4GB to even reach the node backend ?

Kirohá 13 anos

Exactly how did you realize that?

charliesomehá 13 anos

Proof of concept exploit: https://github.com/joyent/node/blob/master/test/simple/test-...

xs_kidhá 13 anos

Your ruby version looks nicer:

https://gist.github.com/anonymous/7050542

meowfacehá 13 anos

Jesus.

If that's all it takes, I have no idea how this wasn't found much sooner.

JoshGlazebrookhá 13 anos

Probably because it's so simple.

esthá 13 anos

while true; do echo -e "GET / HTTP/1.1\r\n\r\n" | nc example.com 80 ; done

[deleted]há 13 anos

hackula1há 13 anos

Give me the TLDR. Being Friday 9pm on the east coast, do I need to go home right now and upgrade my production servers?

AndyKelleyhá 13 anos

Nah. It's a DoS vulnerability, which, in general, you're always vulnerable to. It's just that this one can be done by a single person with relatively low bandwidth.

xs_kidhá 13 anos

Yes, you should do it

kylequesthá 13 anos

It's not just v0.10.21. There's also an update for v0.8.x too (0.8.26): http://blog.nodejs.org/2013/10/18/node-v0-8-26-maintenance/

rdtschá 13 anos

Would putting node behind a proxy like nginx mitigate it?

mathrawkahá 13 anos

With these nginx settings, it does mitigate it for me.

    proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
    proxy_set_header X-Forwarded-Protocol http;
    proxy_set_header Host $http_host;
    proxy_pass http://127.0.0.1:8000;
    proxy_redirect off;

swillshá 13 anos

A CVE would be nice.

mathrawkahá 13 anos

With a pretty default nginx setup as a proxy in front of node, it does not affect earlier versions for me.

[deleted]há 13 anos

lazyjoneshá 13 anos

Someone should benchmark the infamous for its speed node.js again after this (it should be slower)...