CKEditor should protect "onload" and "onerror" attributes

[MaXe Legend]

The following is a security advisory for the Drupal CKEditor. Please note this has also been submitted to the Drupal CKeditor (implementation / plugin) project too.

# Exploit Title: Drupal CKEditor 3.6.2 - Persistent EventHandler XSS # Google Dork: "inurl:"sites/all/modules/ckeditor" -drupalcode.org" # Google Results: Approximately 379.000 results # Date: 9th December 2011 # Author: MaXe @InterN0T # Software Link: ​http://ckeditor.com/ & ​http://drupal.org/node/1332022 # Version: 3.6.2 (Drupal module: 6.x-1.8) # Screenshot: If attached, see image file(s). # Tested on: Windows + FireFox 8.0 & Internet Explorer 8.0

Drupal CKEditor - Persistent / Stored Cross-Site Scripting

Versions Affected: 3.6.2 (Possibly all versions that supports eventhandler injection.) Info: CKEditor is a text editor to be used inside web pages. It's a WYSIWYG editor, which means that the text being edited on it looks as similar as possible to the results users have when publishing it. It brings to the web common editing features found on desktop editing applications like Microsoft Word and OpenOffice. External Links: ​http://ckeditor.com/ ​http://drupal.org/node/1332022 Credits: MaXe (@InterN0T) -:: The Advisory ::- CKEditor is prone to Persistent Cross-Site Scripting within the actual editor, as it is possible for an attacker could maliciously inject eventhandlers serving java- script code in preview / editing in html mode.

If an attacker injects an eventhandler into an image, such as "onload='alert(0);'", then the javascript will execute, even if the data is saved and previewed in editing mode later on. (The XSS will only executing during preview / editing in html mode.)

If an administrator tries to edit the comment afterward, or is logged in and browses to the edit page of the malicious comment, then he or she will execute the javascript, allowing attacker controlled code to run in the context of the browser.

Proof of Concept: Switching to "raw mode" in CKEditor and then writing: <p><img onload="alert(0);" src="​http://1.images.napster.com/mp3s/2348/resources/324/363/files/324363272.jpg" /></p>

Will become this when it is saved: <p><img data-cke-pa-onload="alert(0);" src="​http://1.images.napster.com/mp3s/2348/resources/324/363/files/324363272.jpg" data-cke-saved-src="​http://1.images.napster.com/mp3s/2348/resources/324/363/files/324363272.jpg"></p>

If one searches for alert(0); in Firebug after the code has been injected and executed, the location of the script will be: $full_url_to_script/event/seq/4/onload Where $full_url_to_script is e.g. the following: ​http://localhost/drupal/drupal-6.22/?q=comment/edit/3/event/seq/4/onload

The content of this script is: function onload(event) { alert(0); }

As there is a HTML filter in Drupal, it does not matter whether the <img> tag is allowed in this case, as it was possible to execute the eventhandler either way. (And even store the data.)

-:: Solution ::-

• Awaits developer response *

Note: It shouldn't be possible to use eventhandlers unless explicitly specified by the administrator of the target site. All eventhandler input should also be sanitized / encoded to their equivalent htmlentities and encapsulated in quotes. Disclosure Information: 6th December 2011 - Vulnerability found during a Penetration Test 7th December 2011 - Researched and confirmed the vulnerability 4th January 2012 - Reported to Drupal and CKEditor via ​http://drupal.org/project/ckeditor and ​http://dev.ckeditor.com/ and ​http://cksource.com/contact There has been no public disclosure of this advisory yet. Please respond back to us whenever a solution is available, or if this is deemed a "non-issue".

Reproduced at online demo page at ckeditor.com, changed source content to: <img src="​http://farm4.staticflickr.com/3003/3312196469_7d13c53bdd.jpg" onload="alert(0);" /> and previewed in html mode. Internet Explorer 8.0

Drupal CKEditor - Persistent / Stored Cross-Site Scripting

Versions Affected: 3.6.2 (Possibly all versions that supports eventhandler injection.) Info: CKEditor is a text editor to be used inside web pages. It's a WYSIWYG editor, which means that the text being edited on it looks as similar as possible to the results users have when publishing it. It brings to the web common editing features found on desktop editing applications like Microsoft Word and OpenOffice. External Links: ​http://ckeditor.com/ ​http://drupal.org/node/1332022 Credits: MaXe (@InterN0T) -:: The Advisory ::- CKEditor is prone to Persistent Cross-Site Scripting within the actual editor, as it is possible for an attacker could maliciously inject eventhandlers serving java- script code in preview / editing in html mode.

If an attacker injects an eventhandler into an image, such as "onload='alert(0);'", then the javascript will execute, even if the data is saved and previewed in editing mode later on. (The XSS will only executing during preview / editing in html mode.)

If an administrator tries to edit the comment afterward, or is logged in and browses to the edit page of the malicious comment, then he or she will execute the javascript, allowing attacker controlled code to run in the context of the browser.

Proof of Concept: Switching to "raw mode" in CKEditor and then writing:

[MaXe Legend]

Screenshot

[Frederico Caldeira Knabben]

To make the ticket simple, this is the TC:

1. Load the following HTML in source view:

<img onload="alert(1)" src="http://www.google.com/images/logo.png" />

2. Switch to wysiwyg view.

The alert will be executed.

[Jakub Ś]

Reproducible in all systems from CKEditor 3.0

[Wiktor Walc]

Another similar case with onerror:

<img onerror="alert(1)" src="a" />

[Wiktor Walc]

Since the original TC is long and hard to understand, below is a bit shorter summary.

Drupal handles entered content in a very specific way, displaying it in an unaltered form in editing mode. It means that by design no XSS protection is done on the server side when displaying the text to the administrator in the textarea. Theoretically this is perfectly fine, because by default Drupal is using plain textarea elements (*)

However...

CKEditor (just like other wysiwyg editors, it's not an exception) may be prone to some specific XSS attacks. In fact no wysiwyg editor includes 100% protection against such attacks, especially if we take such browsers like IE. This is because everything that looks like "XSS potection" is done mainly to protect users against executing accidentally JavaScript code, not intentionally to strip all unsafe data. In any case, we're working right now to understand if there's any way to improve this situation.

The very specific way of handling data by Drupal may lead to security issues. Having this thing in mind, the CKEditor module for Drupal runs Drupal filters to make sure that the content loaded into the editor is safe (see the "Security" section in the CKEditor module).

(*) - note that in modern browsers, people can install third-party extensions that can also provide various wysiwyg editors that can be enabled on selected textarea elements, even if no wysiwyg editor is enabled in Drupal. This is again a security risk, because unless every single wysiwyg editor in the world will include a protection against all kind of XSS patterns, users will be prone to this kind of attacks in Drupal.

The ticket is valid, the "onerror" and "onload" attributes should be protected to avoid accidental execution of JavaScript, however that's not a XSS issue in CKEditor itself. CKEditor simply operates on HTML code that is passed to the editor, period (and this is the reason why I changed the title).

[Alfonso Martínez de Lizarrondo]

The problem is at line 520 of htmldataprocessor plugin:

	// Call the browser to help us fixing a possibly invalid HTML
	// structure.
	var div = new CKEDITOR.dom.element( 'div' );
	// Add fake character to workaround IE comments bug. (#3801)
	div.setHtml( 'a' + data );
	data = div.getHtml().substr( 1 );

It sets the contents of a div but at that point the events attributes haven't been protected so they will be executed in that context.

[MaXe Legend]

Please keep in mind that it is all types of eventhandlers that should be filtered, not just onload and onerror. Onmouseover can also be used this way to serve javascript. Any updates on the ticket?

[Alfonso Martínez de Lizarrondo]

I'm not going to state that only onload and onerror can lead to this problems because browsers have too many quirks, but obviously onmouseover doesn't have any effect. Set this HTML and test it:

<img alt="" height="17" onerror="alert(0)" onload="alert(1)" onmouseenter="alert(2)" onmouseleave="alert(3)" onmouseout="alert(4)" onmouseover="alert(5)" src="a.gif" width="15" />

The problem is that the html is being loaded for a moment in a div before getting all the events protected and then loaded in the editable container.

At that moment they are protected as shown in the initial post: data-cke-pa-onload="alert(0);" so any event handler in the original code isn't executed while the document is being edited.

I don't really understand the Drupal decision to keep untrusted HTML as is instead of applying any sanitation when someone else loads it.

[Jakub Ś]

Please have a look at #8584. Perhaps CSP could be used in the future as a fix/extra security to prevent such issues.

[MaXe Legend]

Are you working on getting a fix done? This information has been available to the public (via this bugtracker) for a couple of weeks now. @j.swiderski: It could offer extra security but it is not the solution for the Cross-Site Scripting issue right now. Please let me know as soon as possible.

[Wiktor Walc]

R- because the attached patch does not work in Chrome.

@maxel3g3nd - this issue will be handled like other valid important tickets, however there is no reason to drop everything we're working on at this moment to provide a patch asap. As explained in the roadmap, we are now focused on our next major release. It is a very rare situation where "onload" or "onerror" attributes are used directly in the HTML code to do something in case the image loads or doesn't load.

Please note that if we are talking about 100% protection against XSS, fixing the case described here would not change the overall situation. We're investigating the whole topic to understand what options do we have, but believe me, this is not as simple as it looks like, especially if we take such browsers as IE.

[Alfonso Martínez de Lizarrondo]

If I understand the previous patch properly the idea is to use the fact that scripts aren't executed in an editable iframe, but that premise was broken by webkit long ago and even there's some pressure on Mozilla to make it work in Firefox that way.

The patch that I've attached just performs a simple replacement before and after loading the data in the div and that way the handlers won't be called.

The potential problem with this approach is a performance hit, but this regexps are quite simpler that any other ones used during the load and saving of data, so they shouldn't be a real concern.

[Alfonso Martínez de Lizarrondo]

Simple replacement patch

[MustLive]

Guys!

I've already wrote to MaXe, that I've wrote already about this vulnerability last year. There are two XSS vulnerabilities here: persistent and reflected. Persistent XSS in Drupal - SecurityVulns ID: 11748 (​http://securityvulns.com/docs26584.html and ​http://seclists.org/fulldisclosure/2011/Jun/501).

And similar Reflected XSS in Drupal - SecurityVulns ID: 11750 (​http://securityvulns.com/docs26588.html and ​http://seclists.org/fulldisclosure/2011/Jun/529).

These XSS attacks can be done as via FCKeditor/CKEditor, as via TinyMCE and any other rich editors (with preview functionality). As I've mentioned in publications at my site, these vulnerabilities were found by me at 16.08.2010 (during security audit). After my brief informing about them at 11.12.2010 and detailed informing at 13.04.2011 to Drupal developers, they were ignored and not fixed (so it's no wonder that you've found them). I've announced these vulnerabilities at 12.04.2011 and 13.04.2011, and after giving enough time for developers to fix, they were disclosed at 24.06.2011 and 25.06.2011.

This issue is not in CKEditor itself, but in Drupal (which must properly sanitize the input, if only CKEditor will not have their own XSS filters). So I agree in this with wwalc.

Because these vulnerabilities concern Drupal itself, not only CKEditor (such attack can also be conducted via FCKeditor, TinyMCE and any other rich editors, and it's Drupal's filter fault), I've not informed CKEditor developers, but only Drupal developers. So from MaXe's side, he has did some job to also draw their attention to this issue (and maybe if Drupal is ignoring, then there will be some moving from other side to fix these issues, but it was better for Drupal developers to fix it).

[Frederico Caldeira Knabben]

When using the boundary character match, we have problems when loading contents like this:

<p>Just call element.onerror.</p>

And extra space will be added before "onerror", which is undesirable.

[Frederico Caldeira Knabben]

The patch idea is making it simple though, so I'll be proposing a new patch based on that.

[Frederico Caldeira Knabben]

I have tested the performance of this patch and it has zero impact.

So we must just be sure this will fix this issue and not be worried about performance.

[Garry Yao]

R+ only if the regexp is case-insensitive.

[Frederico Caldeira Knabben]

That's a good point... but simply making the regex case-insensitive is not enough as it'll them break text like " ONE", transforming it to " onE".

A new patch is required.

[Frederico Caldeira Knabben]

Added a few tests for it on t/8630: ​http://ckeditor.t/tt/8630/1.html

[Frederico Caldeira Knabben]

Fixed with [7379].

[Frederico Caldeira Knabben]

With [7413], [7379] has been reverted and a better solution provided.