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 3468503 commit 73009ea
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 2 deletions.
@@ -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
@@ -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
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
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
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
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
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 73009ea

Please sign in to comment.