Make WordPress Core

Opened 4 years ago

Closed 4 months ago

Last modified 4 months ago

#52639 closed enhancement (invalid)

Add proper Security Attributes to the Cookies set by WordPress

Reported by: isaumya's profile 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:

https://i.imgur.com/zbyvDBo.png

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.

https://i.imgur.com/ekz8Qr8.png

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)

52639.png (33.8 KB) - added by SergeyBiryukov 4 years ago.

Download all attachments as: .zip

Change History (12)

#1 in reply to: ↑ description @SergeyBiryukov
4 years ago

  • Keywords reporter-feedback added; needs-patch removed

Hi there, welcome to WordPress Trac! Thanks for the report.

Replying to isaumya:

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.

In my testing, that is already the case:

  • The secure attribute is set based on the is_ssl() value as of WordPress 2.6. Relevant commits:
  • The HttpOnly attribute is set as of WordPress 2.7. Relevant commit:
  • The 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?

@SergeyBiryukov
4 years ago

#2 follow-up: @SergeyBiryukov
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 @isaumya
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.

https://i.imgur.com/RGBRvMO.png

So, why HttpOnly is missing for them?

#4 in reply to: ↑ 2 @isaumya
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 and HttpOnly 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 @isaumya
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:

https://i.imgur.com/KtEZJsq.png

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 @thegulshankumar
4 years ago

All cookies need to be secured. This is how it appears at my end.
Video https://youtu.be/J4dmIvUHepU

#7 @TimothyBlynJacobs
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 @jornfranke
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.

Version 1, edited 2 years ago by jornfranke (previous) (next) (diff)

#9 @TimothyBlynJacobs
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 @andyhirdjt
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 @azaozz
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.

Last edited 4 months ago by azaozz (previous) (diff)
Note: See TracTickets for help on using tickets.