Make WordPress Core

Opened 4 years ago

Last modified 7 days ago

#37000 new enhancement

Support for the SameSite cookie attribute

Reported by: johnbillion Owned by:
Milestone: 5.4 Priority: normal
Severity: normal Version:
Component: Security Keywords: has-patch dev-feedback needs-refresh
Focuses: administration Cc:
PR Number:


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 21 months ago.

Download all attachments as: .zip

Change History (20)

#1 @mwaclawek
4 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@…
3 years ago

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

#4 @demetris
2 years ago

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

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

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

$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


#5 @tomdxw
21 months 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
21 months ago

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

#7 @tomdxw
21 months 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.

21 months ago

#8 @tomdxw
20 months ago

  • Keywords dev-feedback added

#9 @pento
16 months ago

  • Milestone changed from 5.0 to 5.1

#10 @pento
13 months ago

  • Milestone changed from 5.1 to 5.2

#11 @desrosj
10 months ago

  • Keywords needs-refresh added
  • Milestone changed from 5.2 to 5.3

Patch needs to be refreshed to apply cleanly to trunk. 5.2 beta is in less than 2 days. Punting to 5.3.

#12 @ayeshrajans
9 months ago

I'd love to see this move forward too. Even though we commit to a more recent WordPress version, it wouldn't be PHP 7.3 right away, so for BC, we will need a polyfill.

I worked on a quite small scale polyfill (https://github.com/Ayesh/WordPress-SameSite/blob/50705de58a598c46f1ddd70faf6ccce59877b0cc/samesite.php#L172-L188) at just 16 lines and a single function. This function doesn't try to be a complete poyfill, but is a rather auth-cookie exclusive one.

I'd be happy to roll a patch with this smaller polyfill or the delight-im/PHP-Cookie package.

#13 @ayeshrajans
9 months ago

If anyone is interested, I created a plugin to do just that: https://wordpress.org/plugins/samesite

#14 @johnbillion
8 months ago

  • Milestone changed from 5.3 to Future Release

#15 @johnbillion
4 months ago

#48183 was marked as a duplicate.

#16 @mikeschroder
4 months ago

Wanted to be sure the information from #48183 made its way to this ticket.

Chrome is going to set SameSite=lax by default, targeted for Chrome 80:

Looks like that will likely be first quarter next year:

Last edited 4 months ago by mikeschroder (previous) (diff)

#17 @johnbillion
3 months ago

  • Milestone changed from Future Release to 5.4

Moving to 5.4 based on the above.

This ticket was mentioned in Slack in #core by johnbillion. View the logs.

2 months ago

#19 @Toru
7 days ago

Looks like that will likely be first quarter next year:

It is now scheduled for 4 Feb, 2020, which is way before the planned 5.4 date.


Note: See TracTickets for help on using tickets.