Skip to content

Commit

Permalink
Ignore certain data-* attributes in rails-ujs when element is content…
Browse files Browse the repository at this point in the history
…editable

There is a potential DOM based cross-site scripting issue in rails-ujs
which leverages the Clipboard API to target HTML elements that are
assigned the contenteditable attribute. This has the potential to occur
when pasting malicious HTML content from the clipboard that includes
a data-method, data-disable-with or data-remote attribute.

[CVE-2023-23913]
  • Loading branch information
fresh-eggs authored and jhawthorn committed Mar 13, 2023
1 parent 3cf23c3 commit 5037a13
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#= require_tree ../utils

{ matches, getData, setData, stopEverything, formElements } = Rails
{ matches, getData, setData, stopEverything, formElements, isContentEditable } = Rails

Rails.handleDisabledElement = (e) ->
element = this
Expand All @@ -14,6 +14,9 @@ Rails.enableElement = (e) ->
else
element = e

if isContentEditable(element)
return

if matches(element, Rails.linkDisableSelector)
enableLinkElement(element)
else if matches(element, Rails.buttonDisableSelector) or matches(element, Rails.formEnableSelector)
Expand All @@ -24,6 +27,10 @@ Rails.enableElement = (e) ->
# Unified function to disable an element (link, button and form)
Rails.disableElement = (e) ->
element = if e instanceof Event then e.target else e

if isContentEditable(element)
return

if matches(element, Rails.linkDisableSelector)
disableLinkElement(element)
else if matches(element, Rails.buttonDisableSelector) or matches(element, Rails.formDisableSelector)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#= require_tree ../utils

{ stopEverything } = Rails
{ isContentEditable } = Rails

# Handles "data-method" on links such as:
# <a href="/users/5" data-method="delete" rel="nofollow" data-confirm="Are you sure?">Delete</a>
Expand All @@ -9,6 +10,9 @@ Rails.handleMethod = (e) ->
method = link.getAttribute('data-method')
return unless method

if isContentEditable(this)
return

href = Rails.href(link)
csrfToken = Rails.csrfToken()
csrfParam = Rails.csrfParam()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
matches, getData, setData
fire, stopEverything
ajax, isCrossDomain
serializeElement
serializeElement,
isContentEditable
} = Rails

# Checks "data-remote" if true to handle the request through a XHR request.
Expand All @@ -21,6 +22,10 @@ Rails.handleRemote = (e) ->
fire(element, 'ajax:stopped')
return false

if isContentEditable(element)
fire(element, 'ajax:stopped')
return false

withCredentials = element.getAttribute('data-with-credentials')
dataType = element.getAttribute('data-type') or 'script'

Expand Down
12 changes: 12 additions & 0 deletions actionview/app/assets/javascripts/rails-ujs/utils/dom.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,18 @@ Rails.setData = (element, key, value) ->
element[expando] ?= {}
element[expando][key] = value

Rails.isContentEditable = (element) ->
isEditable = false
loop
if(element.isContentEditable)
isEditable = true
break

element = element.parentElement
break unless(element)

return isEditable

# a wrapper for document.querySelectorAll
# returns an Array
Rails.$ = (selector) ->
Expand Down
22 changes: 22 additions & 0 deletions actionview/test/ujs/public/test/data-disable-with.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ module('data-disable-with', {
'data-url': '/echo',
'data-disable-with': 'clicking...'
}))

$('#qunit-fixture').append($('<div />', {
id: 'edit-div', 'contenteditable': 'true'
}))
},
teardown: function() {
$(document).unbind('iframe:loaded')
Expand Down Expand Up @@ -432,3 +436,21 @@ asyncTest('button[data-remote][data-disable-with] re-enables when `ajax:error` e
start()
}, 30)
})

asyncTest('form button with "data-disable-with" attribute and contenteditable is not modified', 6, function() {
var form = $('form[data-remote]'), button = $('<button data-disable-with="submitting ..." name="submit2">Submit</button>')

var contenteditable_div = $('#qunit-fixture').find('div')
form.append(button)
contenteditable_div.append(form)

App.checkEnabledState(button, 'Submit')

setTimeout(function() {
App.checkEnabledState(button, 'Submit')
start()
}, 13)
form.triggerNative('submit')

App.checkEnabledState(button, 'Submit')
})
19 changes: 19 additions & 0 deletions actionview/test/ujs/public/test/data-method.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ module('data-method', {
$('#qunit-fixture').append($('<a />', {
href: '/echo', 'data-method': 'delete', text: 'destroy!'
}))

$('#qunit-fixture').append($('<div />', {
id: 'edit-div', 'contenteditable': 'true'
}))
},
teardown: function() {
$(document).unbind('iframe:loaded')
Expand Down Expand Up @@ -82,4 +86,19 @@ asyncTest('link with "data-method" and cross origin', 1, function() {
notEqual(data.authenticity_token, 'cf50faa3fe97702ca1ae')
})

asyncTest('do not interact with contenteditable elements', 6, function() {
var contenteditable_div = $('#qunit-fixture').find('div')
contenteditable_div.append('<a href="http://www.shouldnevershowindocument.com" data-method="delete">')

var link = $('#edit-div').find('a')
link.triggerNative('click')

start()

collection = document.getElementsByTagName('form')
for (const item of collection) {
notEqual(item.action, "http://www.shouldnevershowindocument.com/")
}
})

})()
20 changes: 20 additions & 0 deletions actionview/test/ujs/public/test/data-remote.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ module('data-remote', {
}))
.find('form').append($('<input type="text" name="user_name" value="john">'))

$('#qunit-fixture').append($('<div />', {
id: 'edit-div', 'contenteditable': 'true'
}))
}
})

Expand Down Expand Up @@ -508,4 +511,21 @@ asyncTest('inputs inside disabled fieldset are not submitted on remote forms', 3
.triggerNative('submit')
})

asyncTest('clicking on a link with contenteditable attribute does not fire ajaxyness', 0, function() {
var contenteditable_div = $('#qunit-fixture').find('div')
var link = $('a[data-remote]')
contenteditable_div.append(link)

link
.bindNative('ajax:beforeSend', function() {
ok(false, 'ajax should not be triggered')
})
.bindNative('click', function(e) {
e.preventDefault()
})
.triggerNative('click')

setTimeout(function() { start() }, 20)
})

})()

0 comments on commit 5037a13

Please sign in to comment.