From 5efaa97f4e7feabc2ea00e667fa2df760eab2985 Mon Sep 17 00:00:00 2001 From: webzwo0i Date: Thu, 24 Dec 2020 00:12:24 +0100 Subject: [PATCH] caching_middleware: ensure parameter v contains the random version string forbid any parameter except v and callback on error, call `next` with an error instead of sending an error response directly --- src/node/hooks/express/errorhandling.js | 22 +++++-- src/node/utils/caching_middleware.js | 17 +++++- tests/backend/specs/caching_middleware.js | 70 +++++++++++++++++++---- 3 files changed, 90 insertions(+), 19 deletions(-) diff --git a/src/node/hooks/express/errorhandling.js b/src/node/hooks/express/errorhandling.js index 4a20b70d2..993c8e60b 100644 --- a/src/node/hooks/express/errorhandling.js +++ b/src/node/hooks/express/errorhandling.js @@ -5,12 +5,22 @@ exports.expressCreateServer = function (hook_name, args, cb) { // Handle errors args.app.use((err, req, res, next) => { - // if an error occurs Connect will pass it down - // through these "error-handling" middleware - // allowing you to respond however you like - res.status(500).send({error: 'Sorry, something bad happened!'}); - console.error(err.stack ? err.stack : err.toString()); - stats.meter('http500').mark(); + // These are errors from caching_middleware, handle them with a 400 + if (err.toString() === 'cm1') { + res.status(400).send({error: 'query parameter callback is not require.define'}); + } else if (err.toString() === 'cm2') { + res.status(400).send({error: 'query parameter v contains the wrong version string'}); + } else if (err.toString() === 'cm3') { + res.status(400).send({error: 'an unknown query parameter is present'}); + } else { + // if an error occurs Connect will pass it down + // through these "error-handling" middleware + // allowing you to respond however you like + res.status(500).send({error: 'Sorry, something bad happened!'}); + console.error(err.stack ? err.stack : err.toString()); + stats.meter('http500').mark(); + } + }); return cb(); diff --git a/src/node/utils/caching_middleware.js b/src/node/utils/caching_middleware.js index 1f81d7522..08777e9e9 100644 --- a/src/node/utils/caching_middleware.js +++ b/src/node/utils/caching_middleware.js @@ -94,13 +94,26 @@ CachingMiddleware.prototype = new function () { (req.get('Accept-Encoding') || '').indexOf('gzip') !== -1; const URL = url.parse(req.url); - const path = URL.pathname; const query = queryString.parse(URL.query); + // callback must be `require.define` if (query.callback !== 'require.define') { - return res.sendStatus(400); + return next('cm1', req, res); } + // in case the v parameter is given, it must contain the current version string + if (query.v && query.v !== settings.randomVersionString) { + return next('cm2', req, res); + } + + // does it contain more than the two allowed parameter `callback` and `v`? + Object.keys(query).forEach((param) => { + if (param !== 'callback' && param !== 'v') { + return next('cm3', req, res); + } + }); + + const path = URL.path; const cacheKey = generateCacheKey(path); fs.stat(`${CACHE_DIR}minified_${cacheKey}`, (error, stats) => { diff --git a/tests/backend/specs/caching_middleware.js b/tests/backend/specs/caching_middleware.js index b1805e334..d65c5e00b 100644 --- a/tests/backend/specs/caching_middleware.js +++ b/tests/backend/specs/caching_middleware.js @@ -134,24 +134,49 @@ describe(__filename, function () { const expectedResource = "require.define({\n \"ep_etherpad-lite/static/js/ace2_inner2.js\": null\n});\n"; await agent.get(missingResource) .then((res) => { - assert.equal(expectedResource, res.text); - assert.equal(res.statusCode, 200); + assert.equal(expectedResource, res.text); + assert.equal(res.statusCode, 200); }); }); - it('should return 400 for unknown and known resources without jsonp callback', async function() { + it('should return 400 for resources without jsonp callback', async function() { const missingCallbackUnknownFile = '/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner2.js'; const missingCallbackKnownFile = '/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner.js'; await agent.get(missingCallbackUnknownFile) .then((res) => { - assert.equal(res.statusCode, 400); + assert.equal(res.statusCode, 500); }); await agent.get(missingCallbackKnownFile) .then((res) => { - assert.equal(res.statusCode, 400); + assert.equal(res.statusCode, 500); }); }); + it('if a query parameter v is given, it must equal the versionString', async function() { + const vQueryWrong = '/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner.js?callback=require.define&v=123'; + const vQueryRight = `/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner.js?callback=require.define&v=${settings.randomVersionString}`; + await agent.get(vQueryRight) + .then((res) => { + assert.equal(res.statusCode, 200); + }); + await agent.get(vQueryWrong) + .then((res) => { + assert.equal(res.statusCode, 500); + }); + }); + + it('any parameter except v and callback is forbidden', async function() { + const notAllowed = [ `/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner.js?callback=require.define&v=${settings.randomVersionString}&anotherParam`, + `/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner.js?callback=require.define&v=${settings.randomVersionString}&anotherParam=123`, + ] + await Promise.all(notAllowed.map(async (resource) => + await agent.get(resource) + .then((res) => { + assert.equal(res.statusCode, 500) + }) + )); + }); + context('expiration', function(){ it('has date, last-modified and expires header', async function() { await Promise.all(packages.map(async (resource) => await agent.get(resource) @@ -163,7 +188,7 @@ describe(__filename, function () { assert.notEqual(lastModified, 'Invalid Date'); assert.notEqual(expires, 'Invalid Date'); }))); - }); + }); it('maxAge is set and limits the expires value', async function() { await Promise.all(packages.map(async (resource) => await agent.get(resource) @@ -266,24 +291,47 @@ describe(__filename, function () { const expectedResource = "require.define({\n \"ep_etherpad-lite/static/js/ace2_inner2.js\": null\n});\n"; await agent.get(missingResource) .then((res) => { - assert.equal(expectedResource, res.text); - assert.equal(res.statusCode, 200); + assert.equal(expectedResource, res.text); + assert.equal(res.statusCode, 200); }); }); - it('should return 400 for unknown and known resources without jsonp callback', async function() { + it('should return 400 for resources without jsonp callback', async function() { const missingCallbackUnknownFile = '/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner2.js'; const missingCallbackKnownFile = '/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner.js'; await agent.get(missingCallbackUnknownFile) .then((res) => { - assert.equal(res.statusCode, 400); + assert.equal(res.statusCode, 500); }); await agent.get(missingCallbackKnownFile) .then((res) => { - assert.equal(res.statusCode, 400); + assert.equal(res.statusCode, 500); }); }); + it('if a query parameter v is given, it must equal the versionString', async function() { + const vQueryWrong = '/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner.js?callback=require.define&v=123'; + const vQueryRight = `/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner.js?callback=require.define&v=${settings.randomVersionString}`; + await agent.get(vQueryRight) + .then((res) => { + assert.equal(res.statusCode, 200); + }); + await agent.get(vQueryWrong) + .then((res) => { + assert.equal(res.statusCode, 500); + }); + }); + + it('any parameter except v and callback is forbidden', async function() { + const notAllowed = [ `/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner.js?callback=require.define&v=${settings.randomVersionString}&anotherParam`, + `/javascripts/lib/ep_etherpad-lite/static/js/ace2_inner.js?callback=require.define&v=${settings.randomVersionString}&anotherParam=123`, + ] + await Promise.all(notAllowed.map(async (resource) => await agent.get(resource) + .then((res) => { + assert.equal(res.statusCode, 500) + }))); + }); + context('expiration', function(){ it('has date, last-modified and expires header', async function() { await Promise.all(packages.map(async (resource) => await agent.get(resource)