Skip to content

Commit

Permalink
[server/shortUrl] filter out invalid target urls
Browse files Browse the repository at this point in the history
  • Loading branch information
spalger committed Nov 4, 2016
1 parent bbd6453 commit 21d2d7f
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 0 deletions.
48 changes: 48 additions & 0 deletions src/server/http/__tests__/short_url_assert_valid.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import Boom from 'boom';

import { shortUrlAssertValid } from '../short_url_assert_valid';


describe('shortUrlAssertValid()', () => {
const invalid = [
['protocol', 'http://localhost:5601/app/kibana'],
['protocol', 'https://localhost:5601/app/kibana'],
['protocol', 'mailto:[email protected]'],
['protocol', 'javascript:alert("hi")'], // eslint-disable-line no-script-url
['hostname', 'localhost/app/kibana'],
['hostname and port', 'local.host:5601/app/kibana'],
['hostname and auth', 'user:[email protected]/app/kibana'],
['path traversal', '/app/../../not-kibana'],
['deep path', '/app/kibana/foo'],
['deep path', '/app/kibana/foo/bar'],
['base path', '/base/app/kibana'],
];

invalid.forEach(([desc, url]) => {
it(`fails when url has ${desc}`, () => {
try {
shortUrlAssertValid(url);
throw new Error(`expected assertion to throw`);
} catch (err) {
if (!err || !err.isBoom) {
throw err;
}
}
});
});

const valid = [
'/app/kibana',
'/app/monitoring#angular/route',
'/app/text#document-id',
'/app/some?with=query',
'/app/some?with=query#and-a-hash',
];

valid.forEach(url => {
it(`allows ${url}`, () => {
shortUrlAssertValid(url);
});
});

});
3 changes: 3 additions & 0 deletions src/server/http/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import HapiStaticFiles from 'inert';
import HapiProxy from 'h2o2';
import getDefaultRoute from './get_default_route';
import versionCheckMixin from './version_check';
import { shortUrlAssertValid } from './short_url_assert_valid';

module.exports = async function (kbnServer, server, config) {

Expand Down Expand Up @@ -117,6 +118,7 @@ module.exports = async function (kbnServer, server, config) {
handler: async function (request, reply) {
try {
const url = await shortUrlLookup.getUrl(request.params.urlId);
shortUrlAssertValid(url);
reply().redirect(config.get('server.basePath') + url);
} catch (err) {
reply(err);
Expand All @@ -129,6 +131,7 @@ module.exports = async function (kbnServer, server, config) {
path: '/shorten',
handler: async function (request, reply) {
try {
shortUrlAssertValid(request.payload.url);
const urlId = await shortUrlLookup.generateUrlId(request.payload.url);
reply(urlId);
} catch (err) {
Expand Down
20 changes: 20 additions & 0 deletions src/server/http/short_url_assert_valid.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { parse } from 'url';
import { trim } from 'lodash';
import Boom from 'boom';

export function shortUrlAssertValid(url) {
const { protocol, hostname, pathname } = parse(url);

if (protocol) {
throw Boom.notAcceptable(`Short url targets cannot have a protocol, found "${protocol}"`);
}

if (hostname) {
throw Boom.notAcceptable(`Short url targets cannot have a hostname, found "${hostname}"`);
}

const pathnameParts = trim(pathname, '/').split('/');
if (pathnameParts.length !== 2) {
throw Boom.notAcceptable(`Short url target path must be in the format "/app/{{appId}}", found "${pathname}"`);
}
}

0 comments on commit 21d2d7f

Please sign in to comment.