From 35328be26b329cfbea6aba49fbbd8b622194c34a Mon Sep 17 00:00:00 2001 From: Zachery Metcalf Date: Wed, 17 Jan 2018 14:03:03 -0700 Subject: [PATCH] Escape strings in HTML output (XSS fix) Modify the template producing HTML error responses to correctly escape all strings that are possibly coming from the client making the request. Before this change, the error responses were vulnerable to XSS (cross-site scripting) attacks. --- .npmrc | 1 + package.json | 2 +- test/handler.test.js | 41 +++++++++++++++++++++++++++++++++++++++++ views/default-error.ejs | 8 ++++---- 4 files changed, 47 insertions(+), 5 deletions(-) create mode 100644 .npmrc diff --git a/.npmrc b/.npmrc new file mode 100644 index 0000000..43c97e7 --- /dev/null +++ b/.npmrc @@ -0,0 +1 @@ +package-lock=false diff --git a/package.json b/package.json index 2d80663..b0a1578 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,7 @@ "dependencies": { "accepts": "^1.3.3", "debug": "^2.2.0", - "ejs": "^2.4.2", + "ejs": "^2.5.7", "http-status": "^1.0.0", "js2xmlparser": "^3.0.0", "strong-globalize": "^3.1.0" diff --git a/test/handler.test.js b/test/handler.test.js index 9dd6c80..5d6b44f 100644 --- a/test/handler.test.js +++ b/test/handler.test.js @@ -472,6 +472,47 @@ describe('strong-error-handler', function() { done); }); + it('HTML-escapes all 4xx response properties in production mode', + function(done) { + const error = new ErrorWithProps({ + name: 'Error', + message: + 'No instance with id found for Model', + statusCode: 404, + }); + givenErrorHandlerForError(error, {debug: false}); + requestHTML() + .end(function(err, res) { + expect(res.statusCode).to.eql(404); + const body = res.error.text; + expect(body).to.match( + /Error<img onerror=alert\(1\) src=a><\/title>/ + ); + expect(body).to.match( + /with id <img onerror=alert\(1\) src=a> found for Model/ + ); + done(); + }); + } + ); + + it('HTML-escapes all 5xx response properties in development mode', + function(done) { + const error = new ErrorWithProps({ + message: 'a test error message<img onerror=alert(1) src=a>', + }); + error.statusCode = 500; + givenErrorHandlerForError(error, {debug: true}); + requestHTML() + .expect(500) + .expect(/<title>ErrorWithProps<\/title>/) + .expect( + /500(.*?)a test error message<img onerror=alert\(1\) src=a>/, + done + ); + } + ); + it('contains subset of properties when status=4xx', function(done) { var error = new ErrorWithProps({ name: 'ValidationError', diff --git a/views/default-error.ejs b/views/default-error.ejs index 26f2d9b..b4069b0 100644 --- a/views/default-error.ejs +++ b/views/default-error.ejs @@ -1,19 +1,19 @@ <html> <head> <meta charset='utf-8'> - <title><%- data.name || data.message %> + <%= data.name || data.message %>
-

<%- data.name %>

-

<%- data.statusCode %> <%- data.message %>

+

<%= data.name %>

+

<%= data.statusCode %> <%= data.message %>

<% // display all the non-standard properties var standardProps = ['name', 'statusCode', 'message', 'stack']; for (var prop in data) { if (standardProps.indexOf(prop) == -1 && data[prop]) { %> -
<%- prop %>: <%- data[prop] %>
+
<%= prop %>: <%= data[prop] %>
<% } } if (data.stack) { %>