From 9d114c7fa61917ad8a87b28411a76efffc8cd311 Mon Sep 17 00:00:00 2001 From: jamesbloomer Date: Sat, 5 Oct 2013 11:17:08 +0100 Subject: [PATCH] Lock down theme static directory to not serve templates, markdown and text files. closes #942 - insert custom middleware to check for blacklisted files - redirect to express.static if file accepted - if not valid return next() to do nothing - currently black listing .hbs, .txt, .md and .json - debatable which is best, black list or white list, either one will probably need tweaks but erred on side of letting a theme serve unknown types --- core/server.js | 5 +- core/server/middleware.js | 31 +++++++++++ core/test/unit/middleware_spec.js | 88 +++++++++++++++++++++++++++++++ 3 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 core/server/middleware.js create mode 100644 core/test/unit/middleware_spec.js diff --git a/core/server.js b/core/server.js index 12f44c44d2..16ffb0c9e4 100644 --- a/core/server.js +++ b/core/server.js @@ -14,6 +14,7 @@ var express = require('express'), hbs = require('express-hbs'), Ghost = require('./ghost'), helpers = require('./server/helpers'), + middleware = require('./server/middleware'), packageInfo = require('../package.json'), // Variables @@ -187,7 +188,7 @@ function activateTheme() { server.set('activeTheme', ghost.settings('activeTheme')); server.enable(server.get('activeTheme')); if (stackLocation) { - server.stack[stackLocation].handle = whenEnabled(server.get('activeTheme'), express['static'](ghost.paths().activeTheme)); + server.stack[stackLocation].handle = whenEnabled(server.get('activeTheme'), middleware.staticTheme(ghost)); } } @@ -259,7 +260,7 @@ when(ghost.init()).then(function () { server.use('/ghost', whenEnabled('admin', express['static'](path.join(__dirname, '/client/assets')))); // Theme only config - server.use(whenEnabled(server.get('activeTheme'), express['static'](ghost.paths().activeTheme))); + server.use(whenEnabled(server.get('activeTheme'), middleware.staticTheme(ghost))); // Add in all trailing slashes server.use(slashes()); diff --git a/core/server/middleware.js b/core/server/middleware.js new file mode 100644 index 0000000000..73833da61f --- /dev/null +++ b/core/server/middleware.js @@ -0,0 +1,31 @@ + +var _ = require('underscore'), + express = require('express'), + path = require('path'); + +function isBlackListedFileType(file) { + var blackListedFileTypes = ['.hbs', '.md', '.txt', '.json'], + ext = path.extname(file); + return _.contains(blackListedFileTypes, ext); +} + +var middleware = { + + staticTheme: function (g) { + var ghost = g; + return function blackListStatic(req, res, next) { + if (isBlackListedFileType(req.url)) { + return next(); + } + + return middleware.forwardToExpressStatic(ghost, req, res, next); + }; + }, + + // to allow unit testing + forwardToExpressStatic: function (ghost, req, res, next) { + return express['static'](ghost.paths().activeTheme)(req, res, next); + } +}; + +module.exports = middleware; diff --git a/core/test/unit/middleware_spec.js b/core/test/unit/middleware_spec.js new file mode 100644 index 0000000000..27324ad7a9 --- /dev/null +++ b/core/test/unit/middleware_spec.js @@ -0,0 +1,88 @@ +/*globals describe, beforeEach, it*/ +var assert = require('assert'), + should = require('should'), + sinon = require('sinon'), + when = require('when'), + express = require('express'), + middleware = require('../../server/middleware'); + +describe('Middleware', function () { + describe('staticTheme', function () { + var realExpressStatic = express.static; + + beforeEach(function () { + sinon.stub(middleware, 'forwardToExpressStatic').yields(); + }); + + afterEach(function () { + middleware.forwardToExpressStatic.restore(); + }); + + it('should call next if hbs file type', function (done) { + var req = { + url: 'mytemplate.hbs' + }; + + middleware.staticTheme(null)(req, null, function (a) { + should.not.exist(a); + middleware.forwardToExpressStatic.calledOnce.should.be.false; + return done(); + }); + }); + + it('should call next if md file type', function (done) { + var req = { + url: 'README.md' + }; + + middleware.staticTheme(null)(req, null, function (a) { + should.not.exist(a); + middleware.forwardToExpressStatic.calledOnce.should.be.false; + return done(); + }); + }); + + it('should call next if txt file type', function (done) { + var req = { + url: 'LICENSE.txt' + }; + + middleware.staticTheme(null)(req, null, function (a) { + should.not.exist(a); + middleware.forwardToExpressStatic.calledOnce.should.be.false; + return done(); + }); + }); + + it('should call next if json file type', function (done) { + var req = { + url: 'sample.json' + } + + middleware.staticTheme(null)(req, null, function (a) { + should.not.exist(a); + middleware.forwardToExpressStatic.calledOnce.should.be.false; + return done(); + }); + }); + + it('should call express.static if valid file type', function (done) { + var ghostStub = { + paths: function() { + return {activeTheme: 'ACTIVETHEME'}; + } + }; + + var req = { + url: 'myvalidfile.css' + }; + + middleware.staticTheme(ghostStub)(req, null, function (req, res, next) { + middleware.forwardToExpressStatic.calledOnce.should.be.true; + assert.deepEqual(middleware.forwardToExpressStatic.args[0][0], ghostStub); + return done(); + }); + }); + }); +}); +