Skip to content
Permalink
Browse files Browse the repository at this point in the history
feat: reject paths with directory traversal
  • Loading branch information
iamtmrobinson committed Mar 18, 2020
1 parent 57c8741 commit 90e0bac
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 8 deletions.
20 changes: 13 additions & 7 deletions lib/filters/index.js
@@ -1,6 +1,7 @@
const minimatch = require('minimatch');
const pathRegexp = require('path-to-regexp');
const qs = require('qs');
const path = require('path');
const undefsafe = require('undefsafe');
const replace = require('../replace-vars');
const authHeader = require('../auth-header');
Expand Down Expand Up @@ -32,7 +33,7 @@ module.exports = ruleSource => {
// array of entries with
const tests = rules.map(entry => {
const keys = [];
let { method, origin, path, valid, stream } = entry;
let { method, origin, path: entryPath, valid, stream } = entry;
method = (method || 'get').toLowerCase();
valid = valid || [];

Expand All @@ -44,27 +45,32 @@ module.exports = ruleSource => {
const fromConfig = {};

// slightly bespoke version of replace-vars.js
path = (path || '').replace(/(\${.*?})/g, (_, match) => {
entryPath = (entryPath || '').replace(/(\${.*?})/g, (_, match) => {
const key = match.slice(2, -1); // ditch the wrappers
fromConfig[key] = config[key] || '';
return ':' + key;
});

origin = replace(origin, config);

if (path[0] !== '/') {
path = '/' + path;
if (entryPath[0] !== '/') {
entryPath = '/' + entryPath;
}

logger.info({ method, path }, 'adding new filter rule');
const regexp = pathRegexp(path, keys);
logger.info({ method, path: entryPath }, 'adding new filter rule');
const regexp = pathRegexp(entryPath, keys);

return (req) => {
// check the request method
if (req.method.toLowerCase() !== method && method !== 'any') {
return false;
}

// Do not allow directory traversal
if (path.normalize(req.url) !== req.url) {
return false;
}

// Discard any fragments before further processing
const mainURI = req.url.split('#')[0];

Expand Down Expand Up @@ -133,7 +139,7 @@ module.exports = ruleSource => {
}
}

logger.debug({ path, origin, url, querystring }, 'rule matched');
logger.debug({ path: entryPath, origin, url, querystring }, 'rule matched');

querystring = (querystring) ? `?${querystring}` : '';
return {
Expand Down
6 changes: 6 additions & 0 deletions test/fixtures/accept/github.json
Expand Up @@ -8,6 +8,12 @@
"method": "GET",
"path": "/repos/:name/:repo/contents/:path*/package.json",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_API}"
},
{
"//": "used to whitelist a folder",
"method": "GET",
"path": "/repos/:name/:repo/contents/:path*/docs/*",
"origin": "https://${GITHUB_TOKEN}@${GITHUB_API}"
}
]
}
14 changes: 13 additions & 1 deletion test/unit/filters.test.js
Expand Up @@ -7,7 +7,7 @@ const jsonBuffer = (body) => Buffer.from(JSON.stringify(body));

test('Filter on URL', t => {
t.test('for GitHub private filters', (t) => {
t.plan(3);
t.plan(4);

const ruleSource = require(__dirname + '/../fixtures/accept/github.json');
const filter = Filters(ruleSource.private);
Expand Down Expand Up @@ -39,6 +39,18 @@ test('Filter on URL', t => {
t.end();
});

t.test('should block when path includes directory traversal', (t) => {
filter({
url: '/repos/angular/angular/contents/path/to/docs/../../sensitive/file.js',
method: 'GET',
}, (error, res) => {
t.equal(error.message, 'blocked', 'has been blocked');
t.equal(res, undefined, 'no follow allowed');
});

t.end();
});

t.end();
});

Expand Down

0 comments on commit 90e0bac

Please sign in to comment.