This post was originally published on Coding Glamour.

Recently we embraced our new 'review for security' strategy, that included a security 101 given by yours truly and a very big messages like 'Don't reinvent the fucking wheel'. Just saying.

http://100procentjan.nl/tweakers/dontwheel.png

Working on this subject intensively also learned me to really really test for security. Developers, and I'm not an exception here, are focussed on code quality, feature stability and security is not always on their radar. And that's a shame because reviewing for security really gives you this 'Dade Murphy' hackers feeling. Meet me at the 'null terminator of death'. Start off by doing some black box testing
A feature came under review that retrieves data from an external source and then offers this info through an HTTP interface. Pretty straight forward. When inspecting the HTTP requests I then see a request per article coming in. That kinda surprise me because I knew the source data is one big blob. There must be some server side processing.

http://100procentjan.nl/tweakers/httprequest.png

Time to try some stuff out. First thing is to randomly change some data and see if it alread 

$ curl http://web.janjongboom.c9.io/api/some/feature/tralalal

Error: ENOENT, open '/var/some/dir/cache/tralalal.json'

Bingo! We now know that there is a cache held on the filesystem and that the file being loaded depends on the URL parameter. Without a white list.

Escaping from cache
The next thing is to escape from the cache folder. Just adding '../' in the path won't magically work, but we can url encode this and see if there is any protection. 

$ curl http://web.janjongboom.c9.io/api/some/feature/..%2server

Error: ENOENT, open '/var/some/dir/server.json'

How great! We don't have to do more weird tricks, and the ../ is even auto resolved for us. Now it's time to start digging around until we are in a more interesting place. If you're reviewing you know the server structure so it should not be hard to move yourself in the source folder of your project. Great thing is that almost all node.js projects have a package.json in their root. That makes the search easier.

This way I could read the package.json of my project. Not so nice. However, I'd rather read some nice information like the private key of the server.

Get around the .json postfix
Let's grab the code. It was something something like: 

app.get('/api/some/feature/:file', function (req, res, next) {
    fs.readFile(__dirname + "/cache/" + req.params.file + ".json", function (err, data) {
        // this causes us to see the full file name
        if (err) return next(err);
        
        res.end(data);
    })
})

So in the construction of the file name, we have already bypassed the '/cache/' and '__dirname' part. But a real hacker should also have the ability to get passed the extension. Together with Bert from the node.js core team we brainstormed a little about ways to do this, and he figured that putting a null terminator (\0) in the file name would probably omit everything thereafter. Makes sense if you realize that all core parts in node are written in C. A simple 'encodeURIComponent("\0")' gives '%00'. Let's just try that... 

$ curl -k /api/some/feature/..%2F..%2F..%2Fhome%2Fubuntu%2F.ssh%2Fid_rsa.pub%00

"Invalid JSON"

Phew, at least there is a JSON.parse that fails. But can you imagine if that check wouldn't be there? The private key of a production server could be stolen. Good thing we do this security reviews.

Lessons learned
  • Reviewing for security always pays off
  • User input is always evil. Escape it. Do a realpath and verify the user should be able to access the file. Use whitelists, etc.
  • The null terminator trick made me really feel hackerish
:9