Make WordPress Core

Opened 15 years ago

Closed 9 years ago

#10237 closed feature request (wontfix)

Implement Content Security Policy to prevent XSS

Reported by: denis-de-bernardy's profile Denis-de-Bernardy Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.8
Component: Security Keywords:
Focuses: Cc:

Description (last modified by dd32)

http://blogs.zdnet.com/security/?p=3654

  1. Here’s how Content Security Policy can provide a way for server administrators to reduce or eliminate their XSS attack surface. Website administrators specify which domains the browser should treat as valid sources of script.
  1. The browser will only execute script in source files from the white-listed domains and will disregard everything else, including inline scripts and event-handling HTML attributes.
    • Note: event-handling is still enabled in CSP without using HTML attributes.
  1. Sites that never want to have JavaScript included in their pages can choose to globally disallow script.

Attachments (3)

10237.patch (513 bytes) - added by hakre 15 years ago.
Content Security Policy (CSP) header in /wp-admin/
csp-wip-v1.patch (35.3 KB) - added by bsterne 15 years ago.
Work in progress. Provides an admin panel to edit CSP and serves header.
everyone.gif (632 bytes) - added by bsterne 15 years ago.
Image referenced by csp-wip-v1.patch

Download all attachments as: .zip

Change History (30)

#1 follow-up: @dd32
15 years ago

  • Description modified (diff)
  • Summary changed from Interesting new feature in Mozilla to prevent XSS to Implement the new Mozilla feature to prevent XSS

Point 2 makes it a bit difficult by the sound of it, Seems to say that no inline JS is allowed, it has to be in a file hosted on a white-listed domain?

Also, Can you find any references on how its implemented? I couldn't see a technical detail anywhere.

#2 in reply to: ↑ 1 @g30rg3x
15 years ago

Replying to dd32:

Also, Can you find any references on how its implemented? I couldn't see a technical detail anywhere.

Content Security Policy Specifications

However according to this message, it will not be implemented until "firefox-next" (after firefox 3.5).

Regards

#3 @hakre
15 years ago

  • Type changed from enhancement to feature request

A took a peek last year on CSP and it's promising. I like the idea WordPress being one of the earlier adopters.

#4 @bsterne
15 years ago

  • Cc bsterne added

I'm available to help implement this feature.

#5 @ryan
15 years ago

  • Milestone changed from Future Release to 3.0

Moving to the 3.0 milestone. Let's see if we can make this happen. Anyone want to get a patch started?

#6 @westi
15 years ago

  • Cc westi added

This looks interesting.

#7 @Denis-de-Bernardy
15 years ago

  • Cc Denis-de-Bernardy added

#8 @hakre
15 years ago

Any Idea if implemented, how it can be verfified that it has been implemented correctly? Is there a validator or something?

@hakre
15 years ago

Content Security Policy (CSP) header in /wp-admin/

#9 @hakre
15 years ago

A little patch but How To Test?!

  1. Get the pre-release of firefox containing Content Security Policy (CSP).
  2. Create yourself a new profile on your desktop if you already have firefox installed (which webdev does not?!) by using the -ProfileManager switch. Keep in mind that firefox must be closed in order that you can launch the profilemanager.
  3. Start the test-browser with your newly created test-profile.
  4. Test. Here is a CSP Testcase.

#10 @hakre
15 years ago

  • Keywords 2nd-opinion added

I personally would love to see some pplz here open to play around a bit with it so more feedback can be practically gathered.

#11 follow-up: @Denis-de-Bernardy
15 years ago

I think that we need a filter for that value. The odds are strong that, this year, someone somewhere will decide to put his js/css on a CDN in order to improve performance. Or a plugin could very well use a js hosted elsewhere (e.g. Google) to do its stuff. Etc.

Restricting this to "self" without any means to override it is a bit too extreme.

#12 @Denis-de-Bernardy
15 years ago

  • Keywords 2nd-opinion removed

#13 in reply to: ↑ 11 ; follow-up: @bsterne
15 years ago

Replying to Denis-de-Bernardy:

Yes, the header added in hakre's patch will be too restrictive for most sites. |allow 'self'| restricts all content, not just scripts, to the same origin as the top level page. Any images or other content coming from other sites will be blocked from loading.

There are other restrictions that need to be considered as well, such as no inline scripts will execute. This means that any event-handling attributes or other inline scripts used in any of the pages, including script added by WP addons, will not execute. This is most immediately a problem in the admin console, which heavily utilizes inline script. All this code will need to be migrated to external script files.

I think we also need to provide a tool in the admin console that allows users to tailor their Content Security Policy according to their site profile. There could be a wizard, for example, that asks questions like "where do you load images from", "do you use any analytics packages", etc., and generates the correct policy based on the answers. It's probably a failure model if we require that users understand the CSP syntax and be writing policies by hand. Of course we can expose the raw policy to users who are comfortable doing so.

As before, I'm very much available to help with this implementation. Let me know how I can be useful.

#14 in reply to: ↑ 13 ; follow-up: @Denis-de-Bernardy
15 years ago

Replying to bsterne:

As before, I'm very much available to help with this implementation. Let me know how I can be useful.

If you're familiar with the specifics, I think that highlighting the headers we should add, with a brief explanation of why, would be of great help. We could easily pick it up from that point.

#15 in reply to: ↑ 14 ; follow-up: @bsterne
15 years ago

Replying to Denis-de-Bernardy:

If you're familiar with the specifics, I think that highlighting the headers we should add, with a brief explanation of why, would be of great help.

Well, no matter what code changes go into the WordPress trunk, it will only require a single header being sent with any given page. It's the value of the header that contains the policy, which can vary somewhat, depending on how the user's blog is set up. I'll give you an example:

If a site needs to allow images from anywhere, videos from YouTube, and everything else from their own host, they would send the header:

X-Content-Security-Policy: allow 'self'; img-src *; media-src: youtube.com

The header value needs to be able to be customized in this way to provide the minimal white-list policy that allows the site to function as intended. There could be a HTML form or some other widget in the admin panel that could help admins construct their policy without directly writing it.

The other set of changes that feature requires is to move any inline script to external script files. This will involve:

  1. changing instances of

<script>inline_code();</script> to

<script src="external_file.js"></script>

These inline code blocks can obviously be combined into a single script file, where appropriate, to reduce the number of network requests.

  1. replace event-handling HTML attributes with externally-added event handlers, such as replacing

<a onclick="commentReply.open('631','101');return false;" class="vim-r hide-if-no-js" title="Reply to this comment" href="#">Reply</a> with

<a class="vim-r hide-if-no-js" cid="631" pid="101" title="Reply to this comment" href="#">Reply</a> with external script doing

var cLinks = document.getElementsByClassName("vim-r");
for (var i = 0 ; i < cLinks.length ; i++) {
  cLinks[i].onclick = commentReply.open(cLinks[i].cid, cLinks[i].pid);
}
  1. We would also want to remove any javascript: URLs, but I didn't see any in the instance of WP I'm working with.

(Also, I cc'd myself on this ticket, but I don't seem to be getting emails. Is there something else I need to do to be notified when things change here?)

#16 in reply to: ↑ 15 @westi
15 years ago

Replying to bsterne:

(Also, I cc'd myself on this ticket, but I don't seem to be getting emails. Is there something else I need to do to be notified when things change here?)

Set your email address here: http://core.trac.wordpress.org/prefs

#17 @bsterne
15 years ago

Replying to bsterne:

  1. replace event-handling HTML attributes with externally-added event handlers, such as replacing

My example code was written hastily. An actual valid example would have been more like:

var cLinks = document.getElementsByClassName("vim-r");
for (var i = 0; i < cLinks.length ; i++) {
  cLinks[i].onclick = function() {
    commentReply.open(this.getAttribute("cid"), this.getAttribute("pid"));
  };
}

#18 @nacin
15 years ago

  • Milestone changed from 3.0 to Future Release

@bsterne
15 years ago

Work in progress. Provides an admin panel to edit CSP and serves header.

@bsterne
15 years ago

Image referenced by csp-wip-v1.patch

#19 @bsterne
15 years ago

I uploaded my work in progress patch adding an administration panel for CSP. It provides a visual way for users to modify their policy and adds the "Suggest Policy" feature which analyzes content in the home page and provides the recommended policy based on those content types and source locations. I'm definitely not a UI expert, so feel free to suggest changes to make it suck less.

The next step in the implementation is to move inline scripts in all of the WP pages into external script files. I'll be working on that shortly.

#20 @bsterne
15 years ago

I finished the CSP implementation as a plugin which you can download and read about here:
http://people.mozilla.org/~bsterne/content-security-policy/wordpress.html

I spoke to westi and nacin over IRC and they suggested a CSP plugin as a proof-of-concept would be valuable here to help people get comfortable with the idea. I encourage anyone following this ticket to try out the CSP plugin and provide feedback.

It's worth noting that the plugin only serves the header for the content portion of the site currently, since there are inline scripts being used in the admin section that can't be moved without a patch to Core.

#21 follow-up: @hakre
15 years ago

Amazing, this is really great news! I just did a testdrive and wrote a review: <a href="http://hakre.wordpress.com/2010/06/01/prevent-xss-on-your-wordpress-blog-with-csp/">Prevent XSS on your wordpress Blog with CSP</a>. I like it so far and might provide some suggestions to it.

#22 in reply to: ↑ 21 ; follow-up: @bsterne
15 years ago

Replying to hakre:
I'd love any feedback you can provide. The only issue I saw you comment on in your review was:

Note 2: Sometimes saving changes does not work with the plugin. Just counter-check by re-opening the admin-page via the menu and save again.

I'm wondering if you're seeing an actual bug, or just a confusing behavior I hadn't figured out good UI for. If you have the actual policy unhidden, then that field will take precedence over any changes you make using the visual editor. Is this what you ran into, or is there actually a problem saving the changes you made?

Thanks in advance for the feedback.

#23 in reply to: ↑ 22 @hakre
15 years ago

Replying to bsterne:

I'm wondering if you're seeing an actual bug, or just a confusing behavior I ![...]

Well I dunno as well. I ran out of time writing the review so I could not inspect that issue more deeply. I hope I find some time on friday to provide a patch as basis to discuss from the stuff that popped into my eyes when using it. But we should discuss that in the plugin trac and/or on the blog instead (or just by email).

#24 @GaryJ
12 years ago

  • Cc gary@… added
  • Keywords needs-patch added
  • Summary changed from Implement the new Mozilla feature to prevent XSS to Implement Content Security Policy to prevent XSS

The patches here (outside of the plugin) would need updating, since the 'allow' string is now 'default-src'. It may also be worth supporting the X-Webkit-CSP header too if it's implemented any time soon.

Would there be a problem if the header was setup and sent at the server level instead of the PHP level?

How much of the inline scripts and styles could be moved to external files for the purpose of adding a CSP header?

It would make the otherwise useful wp_localize_script() redundant unless unsafe-inline was allowed for script-src, and style-src would also need it for gallery, toolbar and other styles.

#25 @ryan
10 years ago

  • Owner ryan deleted
  • Status changed from new to assigned

#26 @chriscct7
9 years ago

Relevant HTTP headers:

  • Content-Security-Policy : Defined by W3C Specs as standard header, used by Chrome version 25 and later, Firefox version 23 and later, Opera version 19 and later.
  • X-Content-Security-Policy : Used by Firefox until version 23, and Internet Explorer version 10 (which partially implements Content Security Policy).
  • X-WebKit-CSP : Used by Chrome until version 25

#27 @johnbillion
9 years ago

  • Keywords needs-patch removed
  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from assigned to closed

I think this is firmly plugin territory. An admin screen for managing CSP is not something that's user friendly to users who don't have a firm understanding of CSP and its consequences.

Adding a default CSP rule to the admin area in WordPress is a separate issue and should be in a separate ticket.

Note: See TracTickets for help on using tickets.