Node crashes when URL path contains nullbyte (%00)

[christophfriedrich]

• [x] Used appropriate template for the issue type
• [x] Searched both open and closed issues for duplicates of this issue
• [x] Title adequately and concisely reflects the feature or the bug

Restify Version: 8.5.1 Node.js Version: 10.19.0 (which is what you get from sudo apt install nodejs in Ubuntu 20.04)

Expected behaviour

Handle error from fs gracefully

Actual behaviour

Crashes the Node process

Repro case

Having installed restify via npm install restify, assume a very basic project that serves a static web page:

testproject
|
 -> server.js
 -> static
    |
     -> index.html

The server.js looks like this:

const restify = require('restify');
var server;
server = restify.createServer();
server.listen(8080, function() {
	console.log('Server started at %s.', server.url);
});
server.get('/*', restify.plugins.serveStatic({
    directory: './static',
    default: 'index.html'
}));

Accessing localhost:8080 serves the contents of the index.html as expected.

But accessing localhost:8080/%00 (note the URL-encoded nullbyte in the end) results in Node crashing with the following error:

internal/fs/utils.js:453 throw err; ^

TypeError [ERR_INVALID_ARG_VALUE]: The argument 'path' must be a string or Uint8Array without null bytes. Received 'static/\u0000' at Object.stat (fs.js:825:3) at serveNormal (/home/cfriedrich/dev/test-restify-nullbyte-bug/node_modules/restify/lib/plugins/static.js:143:12) at serve (/home/cfriedrich/dev/test-restify-nullbyte-bug/node_modules/restify/lib/plugins/static.js:214:13) at nextTick (/home/cfriedrich/dev/test-restify-nullbyte-bug/node_modules/restify/lib/chain.js:167:13) at process._tickCallback (internal/process/next_tick.js:61:11)

Cause

As indicated by the error trace, the cause is the call to fs.stat in static.js:143:

fs.stat(file, function fileStat(err, stats) {

That file argument should be checked for nullbytes before it is fed to fs.stat.

Are you willing and able to fix this?

Probably yes, as adding a condition like

if(file.indexOf("\u0000") != -1)

before the line I quoted above would detect this issue. But I don't know

1. whether that's how you want to approach this and
2. what should happen within that if.

[christophfriedrich]

For now I was able to hotfix this by adding a pre() hook in the server.js:

server.pre((req, res, next) => {
    req.url = req.url.replace(/%00|\u0000/g, '');
    next();
});

[kolbma]

See https://github.com/restify/node-restify/issues/1910