Opened 13 years ago
Last modified 9 years ago
#8584 confirmed New Feature
Support Content Security Policy
Reported by: | nhnb | Owned by: | |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | General | Version: | |
Keywords: | VendorFix | Cc: | rb@…, kevincox@…, enumag@… |
Description
Content Security Policy is a W3D draft aiming to prevent the exploitation of XSS vulnerabilities. It prevents the execution of JavaScript that is directly embedded into HTML code via an inline script element, on-attributes and javascript:-urls. Only external javascript files from a whitelisted domain are executed.
CSP is supported by Firefox since version 4.0 and by the current development versions of webkit. Event the Internet Explorer 10 preview has basic support for CSP.
The main usecsae of CKEditor is to allow users to edit HTML code, which causes a non zero risk of XSS vulnerabilities in either CKEditor itself or the surrounding website. CSP support would be very helpful to mitigate these risks.
Steps to reproduce
- Create a website which uses CKEditor
- Add the following HTTP-Response header. In PHP this is done using the "header" function: X-Content-Security-Policy: default-src 'self'
- Open the page in Firefox > 4.0
Expected Result
CKEditor should work, assuming that it was installed on the same domain as the webpage.
The Firebug extension for Firefox is very helpful because it will list all the violations of the CSP.
Attachments (3)
Change History (28)
comment:2 Changed 13 years ago by
CSP support would be very helpful to mitigate these risks.
@nhnb could you tell me how exactly do you see this support for CSP that CKEditor can provide? Could you give us some examples here?
IMO this is not a bug but feature request.
comment:3 Changed 13 years ago by
Status: | new → pending |
---|
Changed 13 years ago by
Attachment: | standalone.jsp added |
---|
comment:5 Changed 13 years ago by
Keywords: | VendorFix added |
---|---|
Status: | pending → confirmed |
Type: | Bug → New Feature |
This Feature could be used in the future to prevent XSS attacks in CKEditor.
After looking into this issue I have got some results with standalone.jsp sample used in CKEditor for Java. Have a look at the file attached. Just copy it into samples folder and open in a Firefox browser. Before you do that enable Firebug console.
You can also do the same In PHP.
As a result CKEditor should load.
Now change the code from:
<% response.setHeader("X-Content-Security-Policy","allow *; script-src 'self' http://ajax.googleapis.com; options inline-script"); %>
to:
<% response.setHeader("X-Content-Security-Policy","allow *; script-src 'self' options inline-script"); %>
As a result you should see warnings about scripts from google page.
Now change the code to:
<% response.setHeader("X-Content-Security-Policy","allow *; script-src 'self'"); %>
As a result CKEditor should not load at all.
Idealy, it would be great if we could include meta tag in CKEditor iframe to prevent some potentiqal threats. However it is not possible right now. Perhaps this could be used as a future fix/partial fix for the #8630.
Drawbacks:
- Currently only Mozilla Firefox has implemented it.
- Webkit does not support it yet (Chrome 16, Safari 5.1.2). Current Webkit beta versions have some support for CSP.
- IE6-9 don't support it. IE 10 should have some basic support
- Firefox which has best support for CSP does not allow meta tag for CSP although specification mentions it as one of methods of delivery (the worse one). Please see https://bugzilla.mozilla.org/show_bug.cgi?id=663570
I marking this issue as VendorFix and confirming it as it can be implemented in the future.
comment:6 Changed 12 years ago by
When you plan to add CSP support?
<a onclick="CKEDITOR.tools.callFunction(18, this); return false;" onfocus="return CKEDITOR.tools.callFunction(17, event);" onkeydown="return CKEDITOR.tools.callFunction(16, event);" onblur="this.style.cssText = this.style.cssText;" aria-labelledby="cke_12_label" role="button" hidefocus="true" tabindex="-1" title="Drukuj" class="cke_off cke_button_print" id="cke_12"><span class="cke_icon"> </span><span class="cke_label" id="cke_12_label">Drukuj</span></a>
There are inline scripts in CKEditor buttons - onfocus, onkeydown etc. Effect with CSP enabled:
(...) CSP: Directive "inline script base restriction" violated onfocus attribute on A element CSP: Directive "inline script base restriction" violated onkeydown attribute on A element CSP: Directive "inline script base restriction" violated onblur attribute on A element CSP: Directive "inline script base restriction" violated onclick attribute on A element CSP: Directive "inline script base restriction" violated onfocus attribute on A element CSP: Directive "inline script base restriction" violated onkeydown attribute on A element CSP: Directive "inline script base restriction" violated onblur attribute on A element (...)
comment:7 Changed 12 years ago by
No plan to add CSP compatibility?!
The work is simple:
- remove uses of
eval
- convert
on*
attributes to real events (addEvent, removeEvent)
comment:8 Changed 11 years ago by
#10540 was marked as duplictae
Comments from that ticket (step to be made torwards integration):
Reported on GitHub: https://github.com/ckeditor/ckeditor-dev/commit/321ddbbe74a We use eval in env.js and a function constructor in templates. Worth checking if we're not using setTimeout or setInterval with string somewhere in older code.
Below is part of http://www.w3.org/TR/CSP/ spec:
If 'unsafe-eval' is not in allowed script sources:
- Instead of evaluating their arguments, both operator eval and function eval must throw a security exception. [ECMA-262]
- When called as a constructor, the function Function must throw a security exception. [ECMA-262]
- When called with a first argument that is non-callable (e.g., not a function), the setTimeout function must return zero without creating a timer.
- When called with a first argument that is non-callable (e.g., not a function), the setInterval function must return zero without creating a timer.
The term callable refers to an object whose interface has one or more callers as defined in the Web IDL specification [WEBIDL].
comment:9 Changed 11 years ago by
comment:10 Changed 11 years ago by
Cc: | rb@… added |
---|
comment:11 Changed 11 years ago by
Seems that support for CSP is still very limited:
- I anyone wishes to use CKEditor with CSP one has to set both 'unsafe-eval' and 'unsafe-inline' e.g.
response.setHeader("Content-Security-Policy", "default-src 'self'; script-src 'self' 'unsafe-eval' 'unsafe-inline'; style-src 'self' 'unsafe-inline'; frame-src 'self';");
- Users can't disallow eval because it is used in one place in editor - env.js script to test IE
ie: eval( '/*@cc_on!@*/false' ), // Use eval to preserve conditional comment when compiling with Google Closure Compiler (#93).
- Users can't disallow inline scripts as well. Even if all editor creation will be moved to external file, editor still uses some inline script calls. For example when running sample page with above CSP header:
- Firefox will notify us about onmousedown on SPAN element and onblur attribute on A element.
- Chrome will complaint about write function in document.js which uses native window.document.write(...)
- Please also have a look at attached screenshots
I was hoping that at least inline script calls can be avoided but looking at what code is alerted (especially for Chrome) I kind of doubt it will be any time soon.
This is just my personal opinion so if any of you guys have different view on this, please share.
Changed 11 years ago by
Attachment: | Chrome.png added |
---|
Changed 11 years ago by
comment:12 Changed 11 years ago by
Hi,
please let me now, if there is any progress to be exprected on that issue.
I want to add support for CKEditor into the JSF Framework MyFaces Tobago, but I need support for SCP.
The browser support has been grown the last year. See http://caniuse.com/#search=csp. Currently 75% of the browsers out there are supporting CSP at least partially.
comment:13 follow-up: 24 Changed 11 years ago by
Unfortunately there's no way we can add support for all CSP restrictions in CKEditor 4.x. That would require serious editor refactorization or may even be impossible, because we may not be able to achieve some results in other ways than we do currently.
I haven't dug into the CSP yet, but I remember from other ticket that we could get rid of Function
constructor used in templates, what was blocking on of restrictions. If that would at least partially help, let us know - we can consider this change.
However, it will be impossible to get rid of inline event handlers. They are used in various places and are not replaceable with listeners added by JS. An example - some events don't bubble, so delegation is impossible and when we're creating complex and big DOM structures we must use inline handlers, cause otherwise editor initialization would be a lot heavier. Of course it can be solved in other ways, but that would require huge refactorization and deep API changes. I'm also concerned about some hacks which may require using inline event handlers, especially related to iframe loading and COP.
As for plans for CKEditor 5, we'll keep CSP in mind and if it will be possible to have full support for it without serious austerities, then we'll bring it.
comment:14 Changed 10 years ago by
Cc: | kevincox@… added |
---|
comment:15 Changed 10 years ago by
First of all I would love to see this supported. However it is a huge step with how CK is currently designed. I think a good goal would be to remove all inline scripts as a first step. This is already regarded as a good safety and performance decision and shouldn't be too difficult. If CK editor removes all inline scripts then at least that portion of the CSP can be enabled, even if inline CSS has to be allowed.
Inline CSS would be a huge change because of how the editor works. Luckily css is injection is not as harmful as script injection so it can be a lower priority.
comment:16 Changed 10 years ago by
Adding my voice here. CKEditor not supporting CSP means that we have to turn off security for our entire site. Considering that CKEditor is focusing on user-entered html, it seems that XSS protection would be a very high priority...
comment:17 Changed 10 years ago by
As mentioned earlier by @Reinmar the current architecture of CKEditor 4 makes it barely possible to support CSP. However we take this request very seriously and will keep it in mind while working on CKEditor 5. It's being rewritten from scratch which gives us a lot of freedom.
comment:18 Changed 9 years ago by
comment:19 Changed 9 years ago by
Thanks @wwalc. Does this mean that this feature WILL be included in CKEditor 5? (Obviously, I feel it should be a very high priority due to the 'security' aspect) Is there a window for when CKEditor 5 is expected to be released? Not asking for a commitment, but some sort of guess so that we can have a conversation about it internally.
comment:20 Changed 9 years ago by
I see no reason why CKEditor 5 would not include this feature, because this is a nearly complete rewrite, so if this requirement is known from the beginning, there should not be any problems.
As for any ETA - we think about ~2 years from now. But of course first versions won't include all the features that we have now.
comment:22 Changed 9 years ago by
Cc: | enumag@… added |
---|
comment:23 Changed 9 years ago by
The solutions that other popular libraries and frameworks are adopting to comply with CSP are:
1) provide a hook consumers to bypass the indirect evaluation routine (doesn't apply to this case)
2) provide a less performant solution that can be used under certain conditions. I have implemented the corresponding forking logic here:
https://github.com/ckeditor/ckeditor-dev/pull/254
It is very similar to what Angular does for their expression language, and it is a solution that can be ported back to any previous version of CKEditor that relies on the same CKEDITOR.template mechanism.
comment:24 Changed 9 years ago by
Replying to Reinmar:
Unfortunately there's no way we can add support for all CSP restrictions in CKEditor 4.x. That would require serious editor refactorization or may even be impossible, because we may not be able to achieve some results in other ways than we do currently.
I haven't dug into the CSP yet, but I remember from other ticket that we could get rid of
Function
constructor used in templates, what was blocking on of restrictions. If that would at least partially help, let us know - we can consider this change.However, it will be impossible to get rid of inline event handlers. They are used in various places and are not replaceable with listeners added by JS. An example - some events don't bubble, so delegation is impossible and when we're creating complex and big DOM structures we must use inline handlers, cause otherwise editor initialization would be a lot heavier. Of course it can be solved in other ways, but that would require huge refactorization and deep API changes. I'm also concerned about some hacks which may require using inline event handlers, especially related to iframe loading and COP.
As for plans for CKEditor 5, we'll keep CSP in mind and if it will be possible to have full support for it without serious austerities, then we'll bring it.
It is possible to remove inline event handlers - while some events don't bubble, they have equivalents that do bubble - for focus, which I presume is the main issue, focusin can be used instead, as jQuery does. Alternatively, as far as I know, even events that don't bubble will still go through the capturing phase, and can be handled there.
If you replace the inline event handlers with data-cke-onkeydown
etc, then bind a delegate event listener to the top of the editor which then runs CKEDITOR.tools.callFunction on the value of the relevant attribute of the node that the event triggered on, you can emulate the old behaviour very well. For elements that added extra parameters to the function call, just have a data-cke-onkeydown-args
attribute, which should contain an array of parameters to pass, then just JSON decode it and pass it in. In the template, you can just replace ' onclick="CKEDITOR.tools.callFunction(', clickFn, ',null,\'', type, '\');return false;"'
with data-cke-onclick="', clickFn, '" data-cke-onclick-args="', JSON.stringify(null, type), '"'
(from the colorbutton plugin).
You will also need to attach the event listener inside iframes to ensure that they work too, and don't trigger CSP alerts.
Content Security Policy support is really important IMO, and I don't think it's too difficult to enable support.
comment:25 Changed 9 years ago by
Thanks for your feedback. We've been of course considering such solution and it must be doable, but the cost would be huge. CKEditor is 70k LOC + 80k LOC in the tests. Reviewing all the code will be one thing, then you need to update tests, *test manually* all the features, fix bugs, etc, etc. As for testing manually, I know about difference in how focus and focusin behave, because we had many problems with these events in the past. Now, we change the code, we may break everything we polished in the past. There are 10 browsers to check and each behave differently when you push them to the limit (and really, editing is pushing browsers to the limits).
If CKEditor 4 was at the beginning of its life time, we would need to do this change. But in situation when CKEditor 5 is coming and development of CKEditor 4 (and 3 – cause it's the same code base) is slowing down, there's no point in spending months on fixing CSP support. CSP is important, but so do many things, which will cost incomparably less.
Useful links:
https://wiki.mozilla.org/Security/CSP/Specification#Violation_Report_Syntax
https://developer.mozilla.org/en/Security/CSP/Using_Content_Security_Policy
https://developer.mozilla.org/en/Security/CSP/Default_CSP_restrictions
https://dvcs.w3.org/hg/content-security-policy/raw-file/tip/csp-specification.dev.html