Ace: Content Security Policy Support

Created on 5 Apr 2017  路  11Comments  路  Source: ajaxorg/ace

ACE inserts several inline <style> tags into the <head>. For example, the virtual renderer imports ace_editor.css and themes import their own CSS.

This is a problem for pages that want to run a Content Security Policy. Those inline <style> tags must have either a nonce (generated by the server, which ace.js wouldn't possibly know about) or a hash.

I don't have a clear suggestion, but here are some ideas:

  1. Offer a build of ACE that doesn't add any <style> tags to the DOM, but instead exposes them on the filesystem so application authors can include them via <link> tags or @import directives.
  2. Provide some sort of manifest of hashes of all the CSS that ACE provides so application authors can add them to their CSP headers.
  3. Host ACE CSS files on a CDN and add <link> tags referencing them instead of <style> tags.
picture jamesarosen
👍8 ❤1

Most helpful comment

I've calculated hashes for

That silences the errors while initializing the library.

But when ACE renders text, it uses .innerHTML = ... with a string that often includes a style='...' attribute. For example, in ace/layer/marker and several methods in ace/layer/text. These cannot be whitelisted even with a SHA256 hash of the style='...' contents. Additionally, their style='..' contents are dynamically generated, such as in <div class='ace_selection ace_br12' style='height:16px;width:208.83625px;top:64px;left:4px;'> when I click on a row. There's no way to exhaustively enumerate those in a page's CSP.

It may help to create HTMLElements and set their style properties instead of using innerHTML to build up DOM.

picture jamesarosen on 5 Apr 2017
👍2

All 11 comments

Chrome, as of version 46, no longer supports the unsafe-inline option because it's too broad an attack vector, so that isn't an option.

OWASP has some good information about why inline <style> tags are dangerous.

jamesarosen picture jamesarosen on 5 Apr 2017

I've calculated hashes for

That silences the errors while initializing the library.

But when ACE renders text, it uses .innerHTML = ... with a string that often includes a style='...' attribute. For example, in ace/layer/marker and several methods in ace/layer/text. These cannot be whitelisted even with a SHA256 hash of the style='...' contents. Additionally, their style='..' contents are dynamically generated, such as in <div class='ace_selection ace_br12' style='height:16px;width:208.83625px;top:64px;left:4px;'> when I click on a row. There's no way to exhaustively enumerate those in a page's CSP.

It may help to create HTMLElements and set their style properties instead of using innerHTML to build up DOM.

jamesarosen picture jamesarosen on 5 Apr 2017
👍2

The two workarounds for css issue are relatively straightforward to implement, however stopping use of innerHtml is a major change which we can't implement quickly

nightwing picture nightwing on 18 Apr 2017
👎1

however stopping use of innerHtml is a major change which we can't implement quickly

Is that something you'd be interested in, though? It's not something we've prioritized yet, but our team might be able to work on this. Improving XSS protection is very important to us.

jamesarosen picture jamesarosen on 10 Aug 2017
😕1

I have a prototype for removing innerHTML at https://github.com/ajaxorg/ace/pull/3372/files, unfortunately that branch is slower than v1.2.8 when running scroll benchmark from https://github.com/ajaxorg/ace/blob/master/tool/perf-test.html.
If you know a way of optimizing dom calls or know a better way to benchmark, please let us know.

nightwing picture nightwing on 15 Aug 2017

I'll definitely try the use-dom-api branch. Thanks!

If ACE offers CSP support, it would be an advantage over the alternative solutions like CodeMirror.

cgx picture cgx on 15 Aug 2017

If ACE offers CSP support, it would be an advantage over the alternative solutions like CodeMirror.

Absolutely.

Are there any plans to extend the work in https://github.com/ajaxorg/ace/pull/3609 to the marker layer too, @fjakobs?

lol768 picture lol768 on 4 May 2018

@lol768 #3609 removed all uses of innerHTML, now we only need to find a solution for css.

nightwing picture nightwing on 9 May 2018

now we only need to find a solution for css

While style='foo: bar;' is a security risk, el.style.foo = 'bar' should not be and is allowed by even strict Content Security Policies.

jamesarosen picture jamesarosen on 15 May 2018
👍1

@jamesarosen i mean solution for style elements, both providing hashes and creating separate css files require too much changes. Would a solution using nonce-source work for you?

Something like:

"Content-Security-Policy": 
    "default-src 'self' blob: ; style-src 'self' 'nonce-<secretNumber>'; img-src 'self' data:"

ace.config.set("cspToken", <secretNumber>)
ace.edit(...)

so that Ace can set style.nonce when creating the element in https://github.com/ajaxorg/ace/blob/master/lib/ace/lib/dom.js#L191

nightwing picture nightwing on 15 May 2018

From the CSP level 2 spec,

If the policy contains a nonce-source expression, the server MUST generate a fresh value for the nonce-value directive at random and independently each time it transmits a policy. The generated value SHOULD be at least 128 bits long (before encoding), and generated via a cryptographically secure random number generator. This requirement ensures that the nonce-value is difficult for an attacker to predict.

Note: Using a nonce to whitelist inline script or style is less secure than not using a nonce, as nonces override the restrictions in the directive in which they are present. An attacker who can gain access to the nonce can execute whatever script they like, whenever they like. That said, nonces provide a substantial improvement over 'unsafe-inline' when layering a content security policy on top of old code. When considering 'unsafe-inline', authors are encouraged to consider nonces (or hashes) instead.

Because nonces have to be generated on every request, it's not possible to put

ace.config.set("cspToken", <secretNumber>)

in a separate .js file. It would have to be in the including HTML:

<script type='application/javascript' nonce='nonceA'>
ace.config.set('cspToken', nonceB)
</script>

But an attacker who can read ace.config then has access to that value and could use it to execute code on the page.

jamesarosen picture jamesarosen on 16 May 2018
Was this page helpful?
0 / 5 - 0 ratings

Related issues

codeofnode picture codeofnode  路  3Comments
aslushnikov picture aslushnikov  路  4Comments
narraressan picture narraressan  路  3Comments
BoasE picture BoasE  路  4Comments
mafar picture mafar  路  4Comments