inline closing script tag misunderstood by response

[milanchandna]

Hi,

I have a html file which has a script tag something like this:

<script src="/socket.js" type="text/javascript" />

Now I send this html using response to client. I have also tell you that I have taken full care about the content/type to be send.
If it's .js, I am sending content/type as "application/javascript".

Here is snippet:

function handler (request, response) {

//console.log(pathname);
 var filePath = '.' + url.parse(request.url).pathname;
if (filePath == './')
    filePath = './index.htm';
var extname = path.extname(filePath);
var contentType = 'text/html';
switch (extname) {
    case '.js':
        contentType = 'application/javascript';
        break;
    case '.css':
        contentType = 'text/css';
        break;
    case '.png':
        contentType = 'image/png';
        break;
    default :
        contentType = 'text/html';
}
path.exists(filePath, function(exists) {
    if (exists) {
        fs.readFile(filePath, function(error, content) {
            if (error) {
                response.writeHead(500);
                response.end();
            }
            else {
                response.writeHead(200, { 'Content-Type': contentType });
                response.end(content, 'utf-8');
            }
        });
    }
    else {
        response.writeHead(404);
        response.end();
    }
});

//}
}

Now when I receive this html file on client side...it looks something like this.

<script src="/socket.js" type="text/javascript"> ``` <script src="/socket.js" type="text/javascript">

... ... ... </script>

Notice the missing inline closing tag of script on the top and also notice the separate closing tag of script inserted at the end of document.
Because of this the whole HTML body is treated as part of the script.

Though I have found a workaround for now which is to use separate closing script tag rather than using inline
<script src="/socket.js" type="text/javascript" ></script>

Is this a bug in response or in Browser.

I am using Chrome 21.0

Thanks,
Milan.

[bnoordhuis]

Please use the mailing list for questions like this one. The issue tracker is for reporting bugs in node.

[milanchandna]

My bad, sorry for that.

And indeed my mistake, it's not a bug, rather separate closing tag for <script> is always required/recommended.

Thanks anyway.

[isaacs]

@milanchandna Watch out. You have a very bad security flaw in your script.

curl http://host/../etc/passwd
curl http://host/../../etc/passwd
curl http://host/../../../etc/passwd
curl http://host/../../../home/milan/.ssh/id_rsa
... etc

Eventually a clever user can get your http server to disclose machine details.

Instead of var filePath = '.' + url.parse(request.url).pathname;, you should do:

var filePath = '.' + path.join('/', url.parse(request.url).pathname);

so that any leading .. segments will be removed.

[milanchandna]

Hi @isaacs

Many Many Thanks for pointing this out.
I thought over it and yes it looks like a security threat.

But I think the function you used, path.join() will be incapable of removing this threat.

I tried that with something like this:

var filepath = '.' + path.join('/abc/def/../ghi/' + '.');

And when I saw filepath on console, it was '/abc/ghi/'
So I think path.join will not be able to secure it.

I have written a small function to do this, please try and check:

var pathname = url.parse(request.url).pathname;
var pathname_parts = pathname.split('/');
for(var i=0; i<pathname_parts.length; i++) {
    if(pathname_parts[i].indexOf('..') != -1)
        pathname_parts.splice(i,1);
}
pathname = pathname_parts.join('/');
var filePath = './' + pathname;