Skip to content

Commit

Permalink
getSelectorFromElement return null on bad selectors (#27912)
Browse files Browse the repository at this point in the history
  • Loading branch information
Johann-S authored and XhmikosR committed Dec 23, 2018
1 parent 7d5cb2d commit 3bd9fb3
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 32 deletions.
6 changes: 5 additions & 1 deletion js/src/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,11 @@ const Util = {
selector = hrefAttr && hrefAttr !== '#' ? hrefAttr.trim() : ''
}

return selector && document.querySelector(selector) ? selector : null
try {
return document.querySelector(selector) ? selector : null
} catch (err) {
return null
}
},

getTransitionDurationFromElement(element) {
Expand Down
45 changes: 21 additions & 24 deletions js/tests/unit/modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -619,40 +619,37 @@ $(function () {
assert.expect(1)
var done = assert.async()

try {
var $toggleBtn = $('<button data-toggle="modal" data-target="&lt;div id=&quot;modal-test&quot;&gt;&lt;div class=&quot;contents&quot;&lt;div&lt;div id=&quot;close&quot; data-dismiss=&quot;modal&quot;/&gt;&lt;/div&gt;&lt;/div&gt;"/>')
.appendTo('#qunit-fixture')
var $toggleBtn = $('<button data-toggle="modal" data-target="&lt;div id=&quot;modal-test&quot;&gt;&lt;div class=&quot;contents&quot;&lt;div&lt;div id=&quot;close&quot; data-dismiss=&quot;modal&quot;/&gt;&lt;/div&gt;&lt;/div&gt;"/>')
.appendTo('#qunit-fixture')

$toggleBtn.trigger('click')
} catch (e) {
$toggleBtn.trigger('click')
setTimeout(function () {
assert.strictEqual($('#modal-test').length, 0, 'target has not been parsed and added to the document')
done()
}
}, 0)
})

QUnit.test('should not execute js from target', function (assert) {
assert.expect(0)
var done = assert.async()

try {
// This toggle button contains XSS payload in its data-target
// Note: it uses the onerror handler of an img element to execute the js, because a simple script element does not work here
// a script element works in manual tests though, so here it is likely blocked by the qunit framework
var $toggleBtn = $('<button data-toggle="modal" data-target="&lt;div&gt;&lt;image src=&quot;missing.png&quot; onerror=&quot;$(&apos;#qunit-fixture button.control&apos;).trigger(&apos;click&apos;)&quot;&gt;&lt;/div&gt;"/>')
.appendTo('#qunit-fixture')
// The XSS payload above does not have a closure over this function and cannot access the assert object directly
// However, it can send a click event to the following control button, which will then fail the assert
$('<button>')
.addClass('control')
.on('click', function () {
assert.notOk(true, 'XSS payload is not executed as js')
})
.appendTo('#qunit-fixture')
// This toggle button contains XSS payload in its data-target
// Note: it uses the onerror handler of an img element to execute the js, because a simple script element does not work here
// a script element works in manual tests though, so here it is likely blocked by the qunit framework
var $toggleBtn = $('<button data-toggle="modal" data-target="&lt;div&gt;&lt;image src=&quot;missing.png&quot; onerror=&quot;$(&apos;#qunit-fixture button.control&apos;).trigger(&apos;click&apos;)&quot;&gt;&lt;/div&gt;"/>')
.appendTo('#qunit-fixture')
// The XSS payload above does not have a closure over this function and cannot access the assert object directly
// However, it can send a click event to the following control button, which will then fail the assert
$('<button>')
.addClass('control')
.on('click', function () {
assert.notOk(true, 'XSS payload is not executed as js')
})
.appendTo('#qunit-fixture')

$toggleBtn.trigger('click')
} catch (e) {
done()
}
$toggleBtn.trigger('click')

setTimeout(done, 500)
})

QUnit.test('should not try to open a modal which is already visible', function (assert) {
Expand Down
13 changes: 6 additions & 7 deletions js/tests/unit/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,16 @@ $(function () {
assert.strictEqual(Util.getSelectorFromElement($el2[0]), null)
})

QUnit.test('Util.getSelectorFromElement should throw error when there is a bad selector', function (assert) {
QUnit.test('Util.getSelectorFromElement should return null when there is a bad selector', function (assert) {
assert.expect(2)

var $el = $('<div data-target="#1"></div>').appendTo($('#qunit-fixture'))

try {
assert.ok(true, 'trying to use a bad selector')
Util.getSelectorFromElement($el[0])
} catch (e) {
assert.ok(e instanceof DOMException)
}
assert.strictEqual(Util.getSelectorFromElement($el[0]), null)

var $el2 = $('<a href="/posts"></a>').appendTo($('#qunit-fixture'))

assert.strictEqual(Util.getSelectorFromElement($el2[0]), null)
})

QUnit.test('Util.typeCheckConfig should thrown an error when a bad config is passed', function (assert) {
Expand Down

0 comments on commit 3bd9fb3

Please sign in to comment.