abcjs icon indicating copy to clipboard operation
abcjs copied to clipboard

Inline style declarations and Content Security Policy

Open asjl opened this issue 4 years ago • 2 comments

I've been working on various aspects of my website security recently and one area I've been tackling is the use of Content Security Policy (CSP) headers. I've been using https://securityheaders.com/ to test various aspects of the security.

At present because my "style-src" directive does not allow "unsafe-inline" resources to be loaded I'm having some problems with some areas of the abcjs-basic library. I'm using abcjs_basic v6.0.0-beta.33

I'm seeing 3 particular problems:

  1. On line 2968:

var clean_line = encode(line.substring(0, col_num)) + '' + bad_char + '' + encode(line.substring(col_num + 1));

I've resolved this by declaring a CSS class:

.abcjs-abc-warning { text-decoration:underline; font-size:1.3em; font-weight:bold; }

and changing that line to:

var clean_line = encode(line.substring(0, col_num)) + '' + bad_char + '' + encode(line.substring(col_num + 1));

  1. At line 14621:

html += '

CSS required: load abcjs-audio.css
'; html += '\n'; parent.innerHTML = html;

I changed to this:

html += '

CSS required: load abcjs-audio.css
'; html += '\n'; parent.innerHTML = html;
document.getElementById("css-warning").style.cssText = `font-size: 12px;
color:red;
border: 1px solid red;
text-align: center;
width: 300px;
margin-top: 4px;
font-weight: bold;
border-radius: 4px;`;
  1. I'm not sure what to do with this last one as I don't know enough about how SVG works to be able to test a solution.

At line 25075 there's code in the function Svg.prototype.insertStyles that injects some

this.svg.insertBefore(el, this.svg.firstChild); // prepend is not available on older browsers.

It injects this:

<style>.abcjs-dragging-in-progress text, .abcjs-dragging-in-progress tspan {-webkit-touch-callout: none; -webkit-user-select: none; -khtml-user-select: none; -moz-user-select: none; -ms-user-select: none; user-select: none;}</style>

Is it possible to declare CSS classes as above and inject that information to solve the problem.

Note that Chrome says this in the console:

Refused to apply inline style because it violates the following Content Security Policy directive: "style-src 'self' https://*". Either the 'unsafe-inline' keyword, a hash ('sha256-BCpnf71gZdCsSCHDzyB1RsBbZ0qQdqseK6v+yJsgg30='), or a nonce ('nonce-...') is required to enable inline execution.

Firefox also complains:

Content Security Policy: The page’s settings blocked the loading of a resource at inline (“style-src”).

I don't want to set my policy to "unsafe-inline" as that opens the way for other problems.

asjl avatar Aug 02 '21 23:08 asjl

Wow, thanks for the report - I didn't know about this but I'll look into it.

paulrosen avatar Aug 16 '21 00:08 paulrosen

  1. I'm only delivering the js file so there isn't a css file currently loaded. I don't want to include one just for this. I can do the first part of adding the class and document that the client needs to add css if they want.

That's probably a better solution anyway because I'm trying to find places where styles are hardcoded and making them more flexible. The only disadvantage is that the default case is for any user error to not stand out.

  1. I read that style.cssText was also not considered safe, but I can set each individual style similarly to how you show it.

  2. Those styles are inline because they are necessary for dragging to work without inadvertently causing text to be selected. If I just documented that you need to add that to your css or dragging would look funny I doubt anyone would read that. If I were to do a querySelectorAll and set each element every time dragging started or stopped that could be a performance hit. I don't have a ready solution for that.

It would probably work to skip the above code if the options don't include dragging: true. That would save a few bytes in the SVG, too. So it would pass as long as you don't need drag and drop.

paulrosen avatar Aug 16 '21 01:08 paulrosen