From af75502437f3bbdbb18953fd3c45690f635a53bd Mon Sep 17 00:00:00 2001
From: Jeremy Banks <jre@google.com>
Date: Thu, 30 Apr 2015 14:43:40 -0700
Subject: [PATCH] Fixes HTML reflection vulnerability in 500 error status
 handler.

Certain IO errors containing the requested filename could be directly echoed back to the user without a Content-Type header. In some cases, browser sniffing could result in this document being treated as HTML, allowing XSS. This is resolved by sending the Content-Type header of text/plain.

Includes a test case that can reproduce this behaviour on at least some Linux systems.
---
 lib/ecstatic/status-handlers.js              |  2 ++
 test/html-reflection.js                      | 32 ++++++++++++++++++++
 test/public/containsSymlink/more-problematic |  1 +
 3 files changed, 35 insertions(+)
 create mode 100644 test/html-reflection.js
 create mode 120000 test/public/containsSymlink/more-problematic

diff --git a/lib/ecstatic/status-handlers.js b/lib/ecstatic/status-handlers.js
index f64c4b5..9874534 100644
--- a/lib/ecstatic/status-handlers.js
+++ b/lib/ecstatic/status-handlers.js
@@ -60,11 +60,13 @@ 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");
 };
 
 // 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.');
 };
diff --git a/test/html-reflection.js b/test/html-reflection.js
new file mode 100644
index 0000000..4543cdc
--- /dev/null
+++ b/test/html-reflection.js
@@ -0,0 +1,32 @@
+var test = require('tap').test,
+    ecstatic = require('../'),
+    http = require('http'),
+    request = require('request');
+
+var server;
+
+test('html reflection prevented', function (t) {
+  server = http.createServer(ecstatic(__dirname + '/public/containsSymlink'));
+
+  server.listen(0, function () {
+    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.');
+      }
+      t.end();
+    });
+  });
+});
+
+test('server teardown', function (t) {
+  server.close();
+
+  var to = setTimeout(function () {
+    process.stderr.write('# server not closing; slaughtering process.\n');
+    process.exit(0);
+  }, 5000);
+  t.end();
+});
diff --git a/test/public/containsSymlink/more-problematic b/test/public/containsSymlink/more-problematic
new file mode 120000
index 0000000..433c936
--- /dev/null
+++ b/test/public/containsSymlink/more-problematic
@@ -0,0 +1 @@
+/root
\ No newline at end of file