WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 4 weeks ago

#37000 new enhancement

Support for the SameSite cookie attribute

Reported by: johnbillion Owned by:
Milestone: 5.0 Priority: normal
Severity: normal Version:
Component: Security Keywords: has-patch dev-feedback
Focuses: administration Cc:

Description

IETF's Same-site Cookies draft was shipped in Chrome 51 and Opera 39.

The SameSite cookie attribute instructs a browser not to send that cookie with cross-origin third-party requests (such as iframes, embedded images, and Ajax requests). This effectively mitigates CSRF attacks as, for example, the user will not be authenticated for a given third party URL that's being used in a CSRF attack.

More information on the SameSite attribute can be found here: http://www.sjoerdlangkemper.nl/2016/04/14/preventing-csrf-with-samesite-cookie-attribute/

We should investigate whether setting the SameSite=lax attribute is of benefit to the auth and/or logged_in cookies in WordPress, and if so consider implementing it once the draft becomes an RFC.

PHP uses the setcookie() wrapper for setting cookies, which means that setting the SameSite attribute is not possible using that function, until such point that support for the attribute gets added. If WordPress were to implement the SameSite attribute, we'd need our own cookie handling function which constructs and sets the Set-Cookie header itself, and use it in place of setcookie() (side note: this may also be beneficial to unit testing).

Attachments (1)

samesite.diff (20.7 KB) - added by tomdxw 7 weeks ago.

Download all attachments as: .zip

Change History (9)

#1 @mwaclawek
2 years ago

Just a quick note: A compatible and testable replacement for PHP's "setcookie" is available here: https://github.com/delight-im/PHP-Cookie It has support for same-site cookies as well.

Do we really need to wait until the spec is finalized? Chrome has shipped an implementation based on the current draft already, so implementing the current spec would have a real impact on user security immediately (though only for Chrome users).

#2 @noam@…
16 months ago

Related: PHP #72230: Add SameSite Cookies to setcookie() https://bugs.php.net/bug.php?id=72230

#4 @demetris
5 months ago

I just came upon a hack that makes it possible to set the SameSite attribute using the default setcookie() function:

<?php
setcookie('a-cookie', '1', 0, '/; samesite=strict');

I am trying it out on WordPress by passing a modified COOKIEPATH to setcookie():

<?php
$path_plus_samesite = COOKIEPATH . '; samesite=strict';

It seems to work OK on Chrome and on Firefox 59 (the first Firefox version to implement SameSite, currently in the beta channel).

I don’t know whether it would be acceptable to use such a hack in WordPress core, but I thought it would be good to know that it exists.

Link: https://stackoverflow.com/questions/39750906/php-setcookie-samesite-strict

Cheers!

#5 @tomdxw
8 weeks ago

I’ve written a proof of concept plugin which adds SameSite=Lax for all auth cookies: https://gist.github.com/tomdxw/9d7eced5f951680d6eeb52fe6a7a48dc

I tested with a vulnerable plugin, and it works to prevent the CSRF.

The change is tiny - when being merged into core it would require adding one function and modifying 4 lines (the 4 setcookie() lines in wp_set_auth_cookie).

In the very rare situations where a site needs to receive authenticated POST requests, the SameSite cookie can be disabled via filter.

I agree with @mwaclawek. As it’s already been implemented by three of the major browsers (shipped in Chrome, coming to Firefox in May, implemented in WebKit but not released in Safari yet), there’s little chance of substantial change. But it will do a great deal of good by fixing the majority of CSRF vulnerabilities in WordPress plugins (of which there are a lot: https://wpvulndb.com/search?utf8=%E2%9C%93&text=csrf ).

#6 @johnbillion
8 weeks ago

  • Keywords dev-feedback removed
  • Milestone changed from Awaiting Review to 5.0

#7 @tomdxw
7 weeks ago

  • Keywords has-patch added; needs-patch removed

I’ve written a patch for this. I forked the https://github.com/delight-im/PHP-Cookie library suggested by @mwaclawek, and added support for older versions of PHP. It’s here: https://github.com/dxw/PHP-Cookie-Legacy

Only one change was made to src/Cookie.php, and that was to remove the namespace: https://github.com/dxw/PHP-Cookie-Legacy/commit/a5516d70826a56075eb6d452ae5e2028d61cce7c#diff-6c1c5ff819fd1e7697d48e0098012117

The tests are passing with 5.3, 5.4, 5.5, 5.6, 7.0, 7.1, and 7.2: https://travis-ci.org/dxw/PHP-Cookie-Legacy

Due to the tests using php-fpm, and php-fpm being introduced in PHP 5.3, I wasn’t able to get the tests to work with PHP 5.2 (pull requests welcome).

I copied src/Cookie.php (without modification) into WordPress, and modified four lines in wp-includes/pluggable.php. See attached patch.

@tomdxw
7 weeks ago

#8 @tomdxw
4 weeks ago

  • Keywords dev-feedback added
Note: See TracTickets for help on using tickets.