diff --git a/src/js/LayoutManager.js b/src/js/LayoutManager.js index 0c981a7f0..bfe5d38f3 100644 --- a/src/js/LayoutManager.js +++ b/src/js/LayoutManager.js @@ -829,10 +829,11 @@ lm.utils.copy( lm.LayoutManager.prototype, { * @returns {void} */ _adjustToWindowMode: function() { - var popInButton = $( '
' + + var popInButton = $( '
' + '
' + '
' + '
' ); + popInButton.attr( 'title', this.config.labels.popin ); popInButton.click( lm.utils.fnBind( function() { this.emit( 'popIn' ); diff --git a/src/js/controls/HeaderButton.js b/src/js/controls/HeaderButton.js index 2a7349967..468e3cf53 100644 --- a/src/js/controls/HeaderButton.js +++ b/src/js/controls/HeaderButton.js @@ -1,6 +1,6 @@ lm.controls.HeaderButton = function( header, label, cssClass, action ) { this._header = header; - this.element = $( '
  • ' ); + this.element = $( '
  • ' ).attr( { 'class': cssClass, title: label } ); this._header.on( 'destroy', this._$destroy, this ); this._action = action; this.element.on( 'click touchstart', this._action ); diff --git a/test/header-button-tests.js b/test/header-button-tests.js new file mode 100644 index 000000000..11d04904d --- /dev/null +++ b/test/header-button-tests.js @@ -0,0 +1,40 @@ +describe( 'HeaderButton does not allow HTML injection via its label or cssClass', function(){ + var lm = window.GoldenLayout.__lm; + + var makeFakeHeader = function() { + return { + controlsContainer: $( '' ), + on: function(){} + }; + }; + + it( 'stores a malicious label as a literal title attribute without injecting markup', function(){ + var header = makeFakeHeader(), + payload = '">', + button = new lm.controls.HeaderButton( header, payload, 'lm_close', function(){} ); + + // The payload is stored verbatim in the title attribute... + expect( button.element.attr( 'title' ) ).toBe( payload ); + // ...and crucially no extra (e.g. injected ) elements were created. + expect( header.controlsContainer.find( 'img' ).length ).toBe( 0 ); + expect( header.controlsContainer.children().length ).toBe( 1 ); + }); + + it( 'stores a malicious cssClass as a literal class attribute without injecting markup', function(){ + var header = makeFakeHeader(), + payload = '">', + button = new lm.controls.HeaderButton( header, 'a label', payload, function(){} ); + + expect( button.element.attr( 'class' ) ).toBe( payload ); + expect( header.controlsContainer.find( 'img' ).length ).toBe( 0 ); + expect( header.controlsContainer.children().length ).toBe( 1 ); + }); + + it( 'applies a normal label and cssClass correctly', function(){ + var header = makeFakeHeader(), + button = new lm.controls.HeaderButton( header, 'Close', 'lm_close', function(){} ); + + expect( button.element.attr( 'title' ) ).toBe( 'Close' ); + expect( button.element.attr( 'class' ) ).toBe( 'lm_close' ); + }); +} ); diff --git a/test/popin-button-tests.js b/test/popin-button-tests.js new file mode 100644 index 000000000..476a48782 --- /dev/null +++ b/test/popin-button-tests.js @@ -0,0 +1,45 @@ +describe( 'the pop-in button does not allow HTML injection via its label', function(){ + var lm = window.GoldenLayout.__lm; + + /** + * _adjustToWindowMode() is a private method that rebuilds document.body, so we + * call it against a stub `this` and snapshot/restore the document around it + * rather than spinning up a real sub-window (which Karma cannot host - see + * popout-tests.js). + */ + var runAdjustToWindowMode = function( popinLabel ) { + var stub = { + config: { + labels: { popin: popinLabel }, + content: [ { title: 'a title' } ] + }, + emit: function(){} + }, + savedBody = $( 'body' ).children().detach(), + savedTitle = document.title; + + try { + lm.LayoutManager.prototype._adjustToWindowMode.call( stub ); + return stub.container.find( '.lm_popin' ); + } finally { + $( 'body' ).html( '' ).append( savedBody ); + document.title = savedTitle; + delete window.__glInstance; + } + }; + + it( 'stores a malicious label as a literal title attribute without injecting markup', function(){ + var payload = '">', + popInButton = runAdjustToWindowMode( payload ); + + expect( popInButton.length ).toBe( 1 ); + expect( popInButton.attr( 'title' ) ).toBe( payload ); + expect( popInButton.find( 'img' ).length ).toBe( 0 ); + }); + + it( 'applies a normal label correctly', function(){ + var popInButton = runAdjustToWindowMode( 'pop in' ); + + expect( popInButton.attr( 'title' ) ).toBe( 'pop in' ); + }); +} );