diff --git a/client/karma.js b/client/karma.js index 50651037b..9e13d89df 100644 --- a/client/karma.js +++ b/client/karma.js @@ -239,7 +239,15 @@ function Karma (updater, socket, iframe, opener, navigator, location, document) self.updater.updateTestStatus('complete') } if (returnUrl) { - if (!/^https?:\/\//.test(returnUrl)) { + var isReturnUrlAllowed = false + for (var i = 0; i < this.config.allowedReturnUrlPatterns.length; i++) { + var allowedReturnUrlPattern = new RegExp(this.config.allowedReturnUrlPatterns[i]) + if (allowedReturnUrlPattern.test(returnUrl)) { + isReturnUrlAllowed = true + break + } + } + if (!isReturnUrlAllowed) { throw new Error( 'Security: Navigation to '.concat( returnUrl, diff --git a/docs/config/01-configuration-file.md b/docs/config/01-configuration-file.md index ce83ec28c..dff4b6135 100644 --- a/docs/config/01-configuration-file.md +++ b/docs/config/01-configuration-file.md @@ -277,6 +277,15 @@ upon the completion of running the tests. Setting this to false is useful when e If true, Karma does not display the banner and browser list. Useful when using karma on component tests with screenshots. +## client.allowedReturnUrlPatterns +**Type:** Array + +**Default:** `['^https?://']` + +**Description:** Define the string representations of the regular expressions that will be allowed for the `return_url` query parameter. + +If the value of the `return_url` query parameter does not match any regular expression derived from the string representation of each of the elements of this array, navigation to it will be blocked. + ## colors **Type:** Boolean diff --git a/lib/config.js b/lib/config.js index cd9510bfd..240e1c305 100644 --- a/lib/config.js +++ b/lib/config.js @@ -17,19 +17,19 @@ let TYPE_SCRIPT_AVAILABLE = false try { require('coffeescript').register() COFFEE_SCRIPT_AVAILABLE = true -} catch (e) {} +} catch {} // LiveScript is required here to enable config files written in LiveScript. // It's not directly used in this file. try { require('LiveScript') LIVE_SCRIPT_AVAILABLE = true -} catch (e) {} +} catch {} try { require('ts-node') TYPE_SCRIPT_AVAILABLE = true -} catch (e) {} +} catch {} class Pattern { constructor (pattern, served, included, watched, nocache, type, isBinary) { @@ -324,7 +324,8 @@ class Config { useIframe: true, runInParent: false, captureConsole: true, - clearContext: true + clearContext: true, + allowedReturnUrlPatterns: ['^https?://'] } this.browserDisconnectTimeout = 2000 this.browserDisconnectTolerance = 0 diff --git a/static/karma.js b/static/karma.js index f0b2548af..23081eb7f 100644 --- a/static/karma.js +++ b/static/karma.js @@ -249,7 +249,15 @@ function Karma (updater, socket, iframe, opener, navigator, location, document) self.updater.updateTestStatus('complete') } if (returnUrl) { - if (!/^https?:\/\//.test(returnUrl)) { + var isReturnUrlAllowed = false + for (var i = 0; i < this.config.allowedReturnUrlPatterns.length; i++) { + var allowedReturnUrlPattern = new RegExp(this.config.allowedReturnUrlPatterns[i]) + if (allowedReturnUrlPattern.test(returnUrl)) { + isReturnUrlAllowed = true + break + } + } + if (!isReturnUrlAllowed) { throw new Error( 'Security: Navigation to '.concat( returnUrl, diff --git a/test/client/karma.spec.js b/test/client/karma.spec.js index 3e7af73d8..b88ebda38 100644 --- a/test/client/karma.spec.js +++ b/test/client/karma.spec.js @@ -442,15 +442,18 @@ describe('Karma', function () { assert(spyResult.called) }) - it('should navigate the client to return_url if specified', function (done) { + it('should navigate the client to return_url if specified and allowed', function (done) { + var config = { + // The default value. + allowedReturnUrlPatterns: ['^https?://'] + } windowLocation.search = '?id=567&return_url=http://return.com' socket = new MockSocket() k = new ClientKarma(updater, socket, iframe, windowStub, windowNavigator, windowLocation) clientWindow = { karma: k } ck = new ContextKarma(ContextKarma.getDirectCallParentKarmaMethod(clientWindow)) - ck.config = {} + socket.emit('execute', config) - sinon.spy(socket, 'disconnect') clock.tick(500) ck.complete() @@ -462,6 +465,26 @@ describe('Karma', function () { clock.tick(10) }) + it('should not navigate the client to return_url if not allowed', function () { + var config = { + allowedReturnUrlPatterns: [] + } + + windowLocation.search = '?id=567&return_url=javascript:alert(document.domain)' + socket = new MockSocket() + k = new ClientKarma(updater, socket, iframe, windowStub, windowNavigator, windowLocation) + clientWindow = { karma: k } + ck = new ContextKarma(ContextKarma.getDirectCallParentKarmaMethod(clientWindow)) + socket.emit('execute', config) + + try { + ck.complete() + throw new Error('An error should have been caught.') + } catch (error) { + assert(/Error: Security: Navigation to .* was blocked to prevent malicious exploits./.test(error)) + } + }) + it('should clear context window upon complete when clearContext config is true', function () { var config = ck.config = { clearContext: true