Skip to content

Commit

Permalink
Lock down theme static directory to not serve templates, markdown and…
Browse files Browse the repository at this point in the history
… text files.

closes TryGhost#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
  • Loading branch information
jamesbloomer authored and ErisDS committed Oct 11, 2013
1 parent 6db7e6d commit 9d114c7
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 2 deletions.
5 changes: 3 additions & 2 deletions core/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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));
}
}

Expand Down Expand Up @@ -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());
Expand Down
31 changes: 31 additions & 0 deletions core/server/middleware.js
Original file line number Diff line number Diff line change
@@ -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;
88 changes: 88 additions & 0 deletions core/test/unit/middleware_spec.js
Original file line number Diff line number Diff line change
@@ -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();
});
});
});
});

0 comments on commit 9d114c7

Please sign in to comment.