#52639 closed enhancement (invalid)
Add proper Security Attributes to the Cookies set by WordPress
Reported by: | isaumya | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | |
Component: | Security | Keywords: | reporter-feedback |
Focuses: | coding-standards | Cc: |
Description
Hi,
I have come across this security matter while we were developing a project using WordPress as the CMS for the Government. As Government sites go through a strict security audit that's is where I came across this matter.
Apparently, all the cookies that are set by WordPress don't have any of the security attributes like secure
, HttpOnly
, SameSite=Strict
in them. This means these cookies can be accessed any way a script wants.
Not just WP Core but even almost all popular plugins like WooCommerce doesn't follow these security practices, I don't know why. Just adding a few extra parameters to a Cookie can make it quite secure and stop it from being accessed however a script wants.
But here I'm focusing on the WP Core as other plugins look at the coding standards of WP Core and try to follow that. So, implementing these basic security practices in WP Core will lead to many other plugins following the same path and creating a more secure system.
Now if we focus on a very basic /wp-login.php
page:
We can see only the wordpress_test_cookie
has secure
attribute in them while every other cookie simply has Cookie Name, Value, Expiration Date, Max-Age & Path; i.e. only the bare necessary things for that cookie to work and ignoring all other security features. Here is a screenshot:
Instead, WP Core can take advantage of is_ssl()
function to check whether or not to add the secure
attribute. But the rest, i.e. HttpOnly
, SameSite=Strict
should be part of each cookie.
Take a look at WooCommerce cookies now:
I know WooCommerce issue is out of context here as here we are only focusing on WP Core, but I am showing this just to show how widespread this issue is.
So, maybe if WP Core implements these security features it will push others to do the same.
Please note that this is not specific to WP Admin login page but almost all cookies added by WordPress. Would love to see these security attributes get added into cookies added by WP Core.
Attachments (1)
Change History (12)
#1
in reply to:
↑ description
@
4 years ago
- Keywords reporter-feedback added; needs-patch removed
#2
follow-up:
↓ 4
@
4 years ago
I'm also attaching a screenshot of the Chrome Dev Tools panel from an HTTPS site, where you can clearly see the Secure
and HttpOnly
attributes.
#3
@
4 years ago
Hi @SergeyBiryukov,
Thanks a lot for your reply. As you said mentioned above I am glad to know that SameSite
is planned for upcoming future release. And I am glad to know that Secure
is being added to most cookies. But HttpOnly
is still missing on many cookies. For example wordpress_test_cookie
, wp-settings-2
, wp-settings-time-2
etc. don't have HttpOnly
set.
So, why HttpOnly
is missing for them?
#4
in reply to:
↑ 2
@
4 years ago
Replying to SergeyBiryukov:
I'm also attaching a screenshot of the Chrome Dev Tools panel from an HTTPS site, where you can clearly see the
Secure
andHttpOnly
attributes.
Hi @SergeyBiryukov,
Yes but the HttpOnly
is not present in all the cookies added by WP even in your screenshot. Some are still missing it.
Another thing I noticed on /wp-includes/comment.php
on line no. 591
I see this:
$secure = ( 'https' === parse_url( home_url(), PHP_URL_SCHEME ) );
I don't understand why this is being used instead of is_ssl()
. Why have repeating code that does the same thing in a different way?
Also inside /wp-includes/pluggable.php
from line no 987
to 1011
I see a lot of setcookie()
without any $secure
in them.
#5
@
4 years ago
Another wired thing I saw is that when I am visiting lets say /wp-login.php?redirect_to=https%3A%2F%2Fex.example.tech%2Fwp-admin%2F&reauth=1
which is where a page redirects after you just hit /wp-admin/
Among all the cookies I see there only the test cookie has Secure in it. I am looking at the response header of the page:
I have no idea why this is happening. They're supposed to be secure
there at the end. It's a blank testing site on Kinsta. So, there is nothing that can be creating this. It's simply vanilla WP Core. If you want I can share the test site URL.
#6
@
4 years ago
All cookies need to be secured. This is how it appears at my end.
Video https://youtu.be/J4dmIvUHepU
#7
@
4 years ago
For example wordpress_test_cookie, wp-settings-2, wp-settings-time-2 etc. don't have HttpOnly set.
The wp-settings cookies need to be accessible via JS so the wp user settings API works, see utils.js
. The wordpress_test_cookie
is just a test cookie to make sure cookies work. There isn't a security issue from having them accessible over JS.
#8
@
2 years ago
I see this as a clear security issue if the cookie with the session id is available to JS. For instance, a cross site scripting attack can easily steal the cookie and provide it to third parties.
There should be multiple defense mechanisms. HttpOnly and Secure are mandatory for sessions in cookies. There is also no doubt on that (e.g. see here: https://owasp.org/www-community/HttpOnly).
The insecure WP User Setting API needs to be fixed - not by making WordPress at a whole insecure. Organizations may move away from WP if there are easy to configurable defense mechanisms and they are ignored.
It is bad for the reputation of WordPress security if HttpOnly, Secure, SameSite=Strict is not enforced on all WordPress session cookies.
#9
@
2 years ago
I see this as a clear security issue if the cookie with the session id is available to JS. For instance, a cross site scripting attack can easily steal the cookie and provide it to third parties.
The session cookie is not available to JS.
#10
@
4 months ago
Hi there. Similar to previous comments our wordpress based website have had pentests run against them and the testers have raised issues around WP cookie security.
Looking at one of our websites they recommend that the HttpOnly flag is set for the wordpress_test_cookie and other WP cookies where possible.
Additionally the SameSite=Strict should be set on that and admin related cookies.
Thanks!
#11
@
4 months ago
- Milestone Awaiting Review deleted
- Resolution set to invalid
- Status changed from new to closed
May be missing something but looking at the screenshots with the "insecure" cookies, all are expires=Tue, 25-Feb-2020 ...
however this ticket was opened on 02/24/2021, one year later. So unless the screenshots were made exactly one year earlier these all seem to be attempts to delete cookies if they exist, not to set or retrieve them?
As far as I see these "false positive" results are caused by the attempts to delete old WP auth cookies in wp_clear_auth_cookie()
, see https://core.trac.wordpress.org/browser/tags/6.6.1/src/wp-includes/pluggable.php#L1121. Seems the testing software catches these calls and misinterprets them as attempts to set cookies.
Testing in WP 6.6.1 (current release) and trunk/6.7-alpha I don't seem to be able to access any of the mentioned cookies from JS. The only cookie that is accessible is wordpress_test_cookie=WP%20Cookie%20check;
. As explained by @TimothyBlynJacobs above it is designed to work that way and that is not a security concern. All other cookies seem to be properly set to secure, HttpOnly, etc. see https://core.trac.wordpress.org/browser/tags/6.6.1/src/wp-includes/pluggable.php#L1092.
Closing this as invalid as it appears these concerns have been addressed elsewhere. Feel free to reopen if you believe this is still a security concern, i.e. if you can access any of the secure WP cookies from JS.
Hi there, welcome to WordPress Trac! Thanks for the report.
Replying to isaumya:
In my testing, that is already the case:
secure
attribute is set based on theis_ssl()
value as of WordPress 2.6. Relevant commits:HttpOnly
attribute is set as of WordPress 2.7. Relevant commit:SameSite
attribute is indeed not currently added and is planned for a future release, see #37000.It's worth noting that
wp_set_auth_cookie()
is a pluggable function, which means a plugin can redefine and modify it. Could that be the case on your install?