Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential XSS mitigations and encoding corrections #151

Merged
merged 1 commit into from
Sep 14, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion LICENSE.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
The MIT License (MIT)

Copyright (c) 2013 Joshua Holbrook
Copyright (c) 2013-2015 Joshua Holbrook, Jon Ege Ronnenberg, James Halliday,
and Google Inc.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

google?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm making this PR for my work as a Google employee. It was requested that I add this to the copyright notice. As I personally understand things (I am not a lawyer), Google owns the copyright to the few lines being added. I added the largest contributors listed on GitHub to reflect their ownership as well.


Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
Expand Down
6 changes: 3 additions & 3 deletions lib/ecstatic/showdir.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,10 @@ module.exports = function (opts, stat) {
' <head>',
' <meta charset="utf-8">',
' <meta name="viewport" content="width=device-width">',
' <title>Index of ' + pathname +'</title>',
' <title>Index of ' + he.encode(pathname) +'</title>',
' </head>',
' <body>',
'<h1>Index of ' + pathname + '</h1>'
'<h1>Index of ' + he.encode(pathname) + '</h1>'
].join('\n') + '\n';

html += '<table>';
Expand All @@ -124,7 +124,7 @@ module.exports = function (opts, stat) {

// append trailing slash and query for dir entry
if (isDir) {
href += '/' + ((parsed.search)? parsed.search:'');
href += '/' + he.encode((parsed.search)? parsed.search:'');
}

var displayName = he.encode(file[0]) + ((isDir)? '/':'');
Expand Down
40 changes: 36 additions & 4 deletions lib/ecstatic/status-handlers.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
var he = require('he');

// not modified
exports['304'] = function (res, next) {
res.statusCode = 304;
Expand Down Expand Up @@ -60,13 +62,43 @@ exports['416'] = function (res, next) {
// flagrant error
exports['500'] = function (res, next, opts) {
res.statusCode = 500;
res.setHeader('content-type', 'text/plain');
res.end(opts.error.stack || opts.error.toString() || "No specified error");
res.setHeader('content-type', 'text/html');
var error = String(opts.error.stack || opts.error || "No specified error"),
html = [
'<!doctype html>',
'<html>',
' <head>',
' <meta charset="utf-8">',
' <title>500 Internal Server Error</title>',
' </head>',
' <body>',
' <p>',
' ' + he.encode(error),
' </p>',
' </body>',
'</html>'
].join('\n') + '\n';
res.end(html);
};

// bad request
exports['400'] = function (res, next, opts) {
res.statusCode = 400;
res.setHeader('content-type', 'text/plain');
res.end(opts && opts.error ? String(opts.error) : 'Malformed request.');
res.setHeader('content-type', 'text/html');
var error = opts && opts.error ? String(opts.error) : 'Malformed request.',
html = [
'<!doctype html>',
'<html>',
' <head>',
' <meta charset="utf-8">',
' <title>400 Bad Request</title>',
' </head>',
' <body>',
' <p>',
' ' + he.encode(error),
' </p>',
' </body>',
'</html>'
].join('\n') + '\n';
res.end(html);
};
5 changes: 2 additions & 3 deletions test/html-reflection.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@ test('html reflection prevented', function (t) {
var port = server.address().port;
var attack = '<script>alert(\'xss\')</script>';
request.get('http://localhost:' + port + '/more-problematic/' + attack, function (err, res, body) {
if ((!res.headers['content-type'] || res.headers['content-type'] == 'text/html') &&
body.indexOf(attack) != -1) {
t.fail('Unescaped HTML reflected with vulnerable or missing content-type.');
if (body.indexOf('<script>') != -1) {
t.fail('Unescaped HTML reflected.');
}
server.close(function() { t.end(); });
});
Expand Down
Empty file added test/public/<dir>/a.js
Empty file.
2 changes: 1 addition & 1 deletion test/showdir-href-encoding.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ test('url encoding in href', function (t) {
request.get({
uri: uri
}, function(err, res, body) {
t.ok(/href="\/base\/show\-dir\-href\-encoding\/aname\+aplus\.txt"/.test(body), 'We found the right href');
t.match(body, /href="\/base\/show\-dir\-href\-encoding\/aname\+aplus\.txt"/, 'We found the right href');
server.close();
t.end();
});
Expand Down
34 changes: 34 additions & 0 deletions test/showdir-pathname-encoding.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
var test = require('tap').test,
ecstatic = require('../lib/ecstatic'),
http = require('http'),
request = require('request'),
path = require('path');

var root = __dirname + '/public',
baseDir = 'base';

test('directory listing with pathname including HTML characters', function (t) {
var port = Math.floor(Math.random() * ((1<<16) - 1e4) + 1e4);

var uri = 'http://localhost:' + port + path.join('/', baseDir, '/%3Cdir%3E');

var server = http.createServer(
ecstatic({
root: root,
baseDir: baseDir,
showDir: true,
autoIndex: false
})
);

server.listen(port, function () {
request.get({
uri: uri
}, function(err, res, body) {
t.notMatch(body, /<dir>/, 'We didn\'t find the unencoded pathname');
t.match(body, /&#x3C;dir&#x3E;/, 'We found the encoded pathname');
server.close();
t.end();
});
});
});
34 changes: 34 additions & 0 deletions test/showdir-search-encoding.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
var test = require('tap').test,
ecstatic = require('../lib/ecstatic'),
http = require('http'),
request = require('request'),
path = require('path');

var root = __dirname + '/public',
baseDir = 'base';

test('directory listing with query string specified', function (t) {
var port = Math.floor(Math.random() * ((1<<16) - 1e4) + 1e4);

var uri = 'http://localhost:' + port + path.join('/', baseDir, '?a=1&b=2');

var server = http.createServer(
ecstatic({
root: root,
baseDir: baseDir,
showDir: true,
autoIndex: false
})
);

server.listen(port, function () {
request.get({
uri: uri
}, function(err, res, body) {
t.match(body, /href="\/base\/subdir\/\?a=1&#x26;b=2"/, 'We found the encoded href');
t.notMatch(body, /a=1&b=2/, 'We didn\'t find the unencoded query string value');
server.close();
t.end();
});
});
});