Node v0.10.21 Stable has critical security fix
blog.nodejs.org
blog.nodejs.org
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...
- 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.
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.
RealtimeConf: http://2013.realtimeconf.com/
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...
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.
> 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.
Meanwhile, they have the changelog and a test PoC for the one looking to exploit it.
EDIT: Oh and there's an exploit already.
Looks like a flood of concurrent requests will just fill up the memory
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.
Why does that happen? nginx can't help here?
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>
http://andrewkelley.me/post/do-not-use-bodyparser-with-expre...
...joking aside, I'm curious what you saw that made you realize basic flow control was broken?
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?
I didn't think so, but it's starting to seem that way.
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.
If that's all it takes, I have no idea how this wasn't found much sooner.
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;