1833299 - contentEditable elements should strip data-* attributes on paste / drag/drop and similar input

Status: NEW

(Core :: DOM: Editor, enhancement)

Description

[RyotaK]

Attachment: copy.html

VULNERABILITY DETAILS
As reported in https://bugzilla.mozilla.org/show_bug.cgi?id=1599181, the HTML pasting feature of contentEditable is trying to sanitize the contents to prevent UXSS issues.
However, I discovered a case where the protection of this sanitizer can be bypassed.

contentEditable sanitizer allows data-* attributes while sanitizing the pasted contents, but because many libraries/applications are relying on data-* attributes for controlling the behavior/contents of JavaScript/elements, these attributes can often be used to achieve cross-site scripting or similar.

For example, CVE-2023-23913 is a vulnerability that exploits this behavior in rails-ujs. Since rails-ujs uses data-* attributes, such as data-remote or data-confirm, using rails-ujs with contentEditable elements caused a clipboard-based XSS.
https://discuss.rubyonrails.org/t/cve-2023-23913-dom-based-cross-site-scripting-in-rails-ujs-for-contenteditable-html-elements/82468

Also, many libraries allow XSS if the data-* attribute is controllable. (Although exploiting them through contentEditable seems hard for various reasons.)

1. AngularJS: Supports data-ng-* syntax.
2. Angular: Supports data-on-* syntax. (deprecated)
3. Bootstrap: data-html and data-content
4. PrismJS: data-jsonp
5. Knockout.js: data-bind

Because of these reasons, it's better to strip data-* attributes when pasting the contents into contentEditable elements.
That said, I believe it's the responsibility of developers to use these libraries along with contentEditable elements. Still, the issue seems to be affecting a lot of libraries/applications, so I'm reporting it as a security bug before publishing the research just in case browsers want to sanitize "data-" attributes as well. I'll fill similar tickets to Google (Chromium) and Apple (Safari).

(This vulnerability was discovered during the security research that may be published on July 25th.)

VERSION
Chrome Version: 113.0.1 Stable
Operating System: Windows 10 Version 22H2 (Build 19045.2965)

REPRODUCTION CASE

1. Download "copy.html" and "paste.html".
2. Open "copy.html" in the browser, and click the "Copy" button.
3. Open "paste.html" in the browser, and paste the copied contents into the div.
4. Open DevTools and confirm that a div element with data-anything="Hello world!" attribute is created.

CREDIT INFORMATION
Reporter credit: RyotaK

Comment 1

[RyotaK]

Attachment: paste.html

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Removing all data-* attributes may break something of some editable web app libraries which depend on pasted content's attributes. And in strictly speaking, each web app should handle them correctly instead, so it must be the last resort for fixing this in the browser side. Additionally, this can say for all on* attributes too. So I'm afraid regressions of this change. Therefore, if fixing this in the browser side is the only possible choice,

(This vulnerability was discovered during the security research that may be published on July 25th.)

I feel that this due date is too short for considering the big change. E.g., it's impossible to get feedback from testers in a couple of cycles. And unfortunately, 115 will ship in June, and it'll become next ESR, thus, 115 will available until end of next year. However, I don't think that even a security fix for this can be merged into ESR after shipping it.

I'd like to know that how does the security team think about this.

Comment 3

[RyotaK]

I feel that this due date is too short for considering the big change. E.g., it's impossible to get feedback from testers in a couple of cycles. And unfortunately, 115 will ship in June, and it'll become next ESR, thus, 115 will available until end of next year. However, I don't think that even a security fix for this can be merged into ESR after shipping it.

I can delay/cancel the publishment of the research if this issue is considered a security bug, and browsers need to have time to fix it. So please let me know if it's needed.

Comment 4

[RyotaK]

Additionally, this can say for all on* attributes too.

Do contentEditable elements allow inline event handlers? From what I tested, it's not allowing on* attributes because it'd cause UXSS. However, if you could paste on* attributes into the contentEditable field, that may be another bug.
(Sorry for splitting comments, I completely missed it while writing previous comment.)

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Ah, on* are not copied within current Nightly. So, I was wrong, sorry for the spam. Note that contenteditable can be set to false and the <div contenteditable>.innerHTML may be used for preview or output as non-editable content. Therefore, any attributes which run JS are not safe to paste into editable element.

Comment 6

[Andrew McCreight [:mccr8]]

In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #2)

However, I don't think that even a security fix for this can be merged into ESR after shipping it.

We backport security fixes to ESR, and new minor versions of ESR are released at the same cadence as regular releases, so that shouldn't be an issue.

Comment 7

[Daniel Veditz [:dveditz]]

Freddy: any thoughts on this one? would it be feasible to register a sanitizer that would get called for pastes? But what web devs will know to use such a thing? Thoughts about a rating on this?

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

(In reply to Andrew McCreight [:mccr8] from comment #6)

In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #2)

However, I don't think that even a security fix for this can be merged into ESR after shipping it.

We backport security fixes to ESR, and new minor versions of ESR are released at the same cadence as regular releases, so that shouldn't be an issue.

Fixing this bug needs non-trivial behavior change (compared with current and traditional behavior). Therefore, once we ship ESR, it's hard to fix this in the branch. And also once we ship ESR with fixing this, and then, we meet a serious breakage of other web apps, it's hard to back it out from the branch because it changes the behavior.

Comment 9

[Frederik Braun [:freddy]]

The issue with data- attributes is that their meaning is heavily dependent on the application or library they are being used in.
I know there are libraries where data-text is totally safe and should be best kept to make sure the application keeps working and then there are cases where data-text is automatically transformed using element.innerHTML=, which is super unsafe.

Research similar to this reported here has also been called "Script Gadgets", as published by the Google Security team.

For this specific bug: RyotaK did you perform cross-browser analysis of how other vendors treat this kind of issue? I'd be willing to work on a spot fix, if we had confidence that this is along the lines of what others are already doing. I don't think it would be wise to step out of line for this kind of attack.

Comment 10

[Frederik Braun [:freddy]]

For the bigger picture:
Realistically, I don't think we can strip data- attributes in a manner that may risk break existing applications without further testing. Especially with very little specified behavior across browsers about what content should be kept or stripped generally.

I think our only way out here is to standardize browser sanitization in general, which we are working on as part of the Sanitizer API. This still requires broad cross-browser agreement on reasonable defaults, which we are currently working on right now and specifically touches on the issue of data- attributes in last week's meeting.

This will take some time and once all of these things have been resolved, I'd be hopeful of having the sanitization happening as part of copy & paste become standardized on top of the Sanitizer API.

Comment 11

[RyotaK]

(In reply to Frederik Braun [:freddy] from comment #9)

The issue with data- attributes is that their meaning is heavily dependent on the application or library they are being used in.
I know there are libraries where data-text is totally safe and should be best kept to make sure the application keeps working and then there are cases where data-text is automatically transformed using element.innerHTML=, which is super unsafe.

Research similar to this reported here has also been called "Script Gadgets", as published by the Google Security team.

For this specific bug: RyotaK did you perform cross-browser analysis of how other vendors treat this kind of issue? I'd be willing to work on a spot fix, if we had confidence that this is along the lines of what others are already doing. I don't think it would be wise to step out of line for this kind of attack.

Yes, I performed the cross-browser analysis on this and confirmed that Chromium / Safari also accepts data- attributes in the contentEditable elements. However, since I filled this ticket at the same time I filled their tickets, they are also still discussing this case, and as far as I know, they haven't mitigated/prevented it.
For the reference, issue 1445815 for Chromium and report OE1940602292615 for Safari. (Both are still private, so you might be unable to access them.)

Comment 12

[Frederik Braun [:freddy]]

OK, looks like this is a shortcoming of the web standards. I will reach out to our contacts at Google and Apple to see if they have any concerns in making this bug public and continuing this conversation as a group. This might take a while.

Comment 13

[Daniel Veditz [:dveditz]]

I think our only way out here is to standardize browser sanitization in general, which we are working on as part of the Sanitizer API. ... specifically touches on the issue of data- attributes in last week's meeting.

Currently nothing in the Sanitizer is hooked into copy/paste. Site's could capture the paste event, run that through the sanitizer, and then perform the paste manually, but it also might be super nice to invent a way to let webdevs hook the sanitizer into copy/paste automatically so they didn't have to mess with that.

Comment 14

[Frederik Braun [:freddy]]

(In reply to Frederik Braun [:freddy] from comment #12)

OK, looks like this is a shortcoming of the web standards. I will reach out to our contacts at Google and Apple to see if they have any concerns in making this bug public and continuing this conversation as a group. This might take a while.

Anne, can you help us here?

Comment 15

[RyotaK]

Google and Apple decided not to fix this issue, so there should be no problems publishing this issue.
As a side note, Chromium's implementation is much more permissive as it uses the deny-list approach for sanitizing attributes. (i.e., non-standard attributes are allowed)