From 21d2d7fdd24d13e674b877b3fa497e04d50c030e Mon Sep 17 00:00:00 2001 From: spalger Date: Mon, 31 Oct 2016 14:03:35 -0700 Subject: [PATCH] [server/shortUrl] filter out invalid target urls --- .../http/__tests__/short_url_assert_valid.js | 48 +++++++++++++++++++ src/server/http/index.js | 3 ++ src/server/http/short_url_assert_valid.js | 20 ++++++++ 3 files changed, 71 insertions(+) create mode 100644 src/server/http/__tests__/short_url_assert_valid.js create mode 100644 src/server/http/short_url_assert_valid.js diff --git a/src/server/http/__tests__/short_url_assert_valid.js b/src/server/http/__tests__/short_url_assert_valid.js new file mode 100644 index 0000000000000..5bedf0959a790 --- /dev/null +++ b/src/server/http/__tests__/short_url_assert_valid.js @@ -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:foo@bar.net'], + ['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:pass@localhost.net/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); + }); + }); + +}); diff --git a/src/server/http/index.js b/src/server/http/index.js index f8d9df1512cd7..2f84533923d9d 100644 --- a/src/server/http/index.js +++ b/src/server/http/index.js @@ -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) { @@ -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); @@ -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) { diff --git a/src/server/http/short_url_assert_valid.js b/src/server/http/short_url_assert_valid.js new file mode 100644 index 0000000000000..b48ef5853f662 --- /dev/null +++ b/src/server/http/short_url_assert_valid.js @@ -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}"`); + } +}