#16483 closed defect (bug) (wontfix)
Visibility: password-protected exposes multiple pages
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 3.0.4 |
Component: | Security | Keywords: | dev-feedback needs-testing has-patch |
Focuses: | Cc: |
Description
- password protect a page ('protected') with a password
- password protect another page ('thistoo') with the SAME password
- visit 'protected' and enter the password. Page is visible
- visit 'thistoo'; expected: prompt for password. What happens: Page is visible
Regardless of whether someone with a password has the right to try it in as many pages as they want (and would therefore successfully see the page if the passwords were the same), the user should still be prompted on a page-by-page basis. Global authentication to multiple pages is possible with user accounts and roles. It should not be possible with visibility: password-protected pages.
Attachments (8)
Change History (50)
#1
@
14 years ago
- Keywords has-patch dev-feedback added
I agree that this shouldn't happen - although plain text passwords in cookies aren't really going to prevent someone who is determined ;)
Here's one possible patch. It may cause some issues because previously (and since WP 1.0.0) get_the_password_form()
didn't require a post. I can't find any instances in core that don't have a post (or implicit post global) set, but I might have missed something.
#2
@
12 years ago
- Cc mdhansen@… added
- Keywords needs-refresh added
This problem still exists in 3.5-RC1-22924. Patch needs refresh.
#4
follow-up:
↓ 5
@
12 years ago
16483.3.diff does not seem to let me log in to the page at all even with the correct password.
#5
in reply to:
↑ 4
@
12 years ago
Replying to MikeHansenMe:
16483.3.diff does not seem to let me log in to the page at all even with the correct password.
Thanks, fixed in 16483.4.diff. Missed [19728] in the previous patch.
#6
@
12 years ago
- Component changed from General to Security
- Milestone changed from Awaiting Review to 3.6
Related: #19797
#7
@
12 years ago
- Milestone changed from 3.6 to Future Release
This has been discussed a number of times and there are many who consider this a feature. Punting back to Future Release since it requires negotiations.
#8
follow-up:
↓ 9
@
11 years ago
I just had a use case/question on WP.SE where this "feature" was actually wanted. But I agree that it is the opposite of ideal.
Keep in mind that #20308 will allow to query by has_password
as well as by post_password
. So this ticket will likely go against the new feature for WP_Query
arguments.
#9
in reply to:
↑ 8
@
11 years ago
Replying to F J Kaiser:
Keep in mind that #20308 will allow to query by
has_password
as well as bypost_password
. So this ticket will likely go against the new feature forWP_Query
arguments.
Yes and no. There are two options:
- Use one cookie not keyed to a post, which means multiple posts can be accessed at once after entering a single password, but a post with a different password cannot be accessed without overwriting the cookie. (Current.)
- Use a cookie keyed to each post, which means multiple posts can be viewed even if they don't have the same password. Every post causes a prompt even if they have the same password. (Proposed.)
I wouldn't mind a way to toggle between these states. Really, what we need is a filter on the cookie name, right?
But in both cases, multiple posts having the same password are still linked in some regard: the user has a single password that unlocks one or more posts. Whether they need to enter it again or not doesn't really affect how WP_Query can now query for post passwords. I would be OK with ignoring that API change for the purposes of making a decision here.
#10
@
9 years ago
- Keywords needs-refresh added; has-patch removed
- Severity changed from minor to normal
#11
@
9 years ago
- Milestone changed from Future Release to 4.7
I would love to get this into 4.7. This "feature" could have some pretty nasty consequences for an unsuspecting site admin.
#13
follow-up:
↓ 14
@
8 years ago
Could I please have a summary of the following, both for myself and for general reference?
- What is the proposed solution in the patch?
- How does this affect existing post passwords?
- How would original functionality be restored via plugin?
- How does this relate to
WP_Query
changes in #20308 cited above? - How does this affect the REST API? See https://github.com/WP-API/WP-API/issues/1055
#14
in reply to:
↑ 13
@
8 years ago
Replying to helen:
Could I please have a summary of the following, both for myself and for general reference?
- What is the proposed solution in the patch?
The proposed solution is to alter the cookie to be post specific, as opposed to password specific. Currently, any posts with the same password can be viewed at the same time.
- How does this affect existing post passwords?
As far as I am aware, this would invalidate all current cookies and force everyone to re-authenticate to any protected posts. I consider this a non-issue for a few reasons. Cookies can accidentally be deleted, aren't available across all devices, clearing browser history often deletes them, etc, etc. The are so many cases for cookies disappearing this, to me, doesn't seem like a big deal.
- How would original functionality be restored via plugin?
In the proposed solution, it wouldn't.
- How does this relate to
WP_Query
changes in #20308 cited above?
I currently don't know the answer to this question, but I will find out.
- How does this affect the REST API? See https://github.com/WP-API/WP-API/issues/1055
After reading the REST API issue, it seems like it would make life a little better for them. It has been noted a couple times that an "ugly nuance of core" is that its cookie based and only based on COOKIEHASH, so only one password protected post can be viewed at a time. This fixes that so multiple password protected posts can be viewed.
#15
follow-up:
↓ 16
@
8 years ago
After reading #20308, the only way I can see that effecting anything is if a developer does the following:
Checks the protected posts cookie if it exists.
If it exists, only show posts that have that exact password.
However, the query would still be able to list all posts with the same password. This would just require users to enter the password for each post. I'm struggling to come up with a scenario where this would be the solution though.
Either way, the remedy to this, if its even an issue, could be doing what @nacin suggested and adding a filter to the cookie name. That would allow a plugin to restore this "feature".
#16
in reply to:
↑ 15
@
8 years ago
Replying to voldemortensen:
How would original functionality be restored via plugin?
In the proposed solution, it wouldn't.
Either way, the remedy to this, if its even an issue, could be doing what @nacin suggested and adding a filter to the cookie name. That would allow a plugin to restore this "feature".
This should be accounted for in whatever solution is implemented.
This ticket was mentioned in Slack in #core-restapi by helen. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by stevenkword. View the logs.
8 years ago
#20
@
8 years ago
- Keywords close added
Changing this is backwards compatibility break that I'm not really a fan of.
The new filter introduced in [38603] allows for changing the password protected post behavior on any sites that are interested in doing so.
I think we can close this as wontfix. The behavior where one password can open up multiple posts is a feature at this point.
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
8 years ago
#22
@
8 years ago
- Keywords close removed
I'm personally fine with breaking existing behavior - it is very unexpected (i.e. a bug) and nothing in screen help or even the codex warns you about the shared password behavior. I think people who are using it on purpose are likely to know WP fairly well and can install a plugin to restore it - the worst that happens in the meantime to their readers is they re-enter that same password which is ostensibly a known password.
If we don't change it, I think we need a review of copy around this feature, so still wouldn't close the ticket.
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
8 years ago
#26
@
8 years ago
Please ignore 16483.7.diff. grunt-patch-wordpress did something I didn't expect.
16483.8.diff adds a filter to the cookie name so that previous functionality can be restored.
PoC mu-plugin to restore functionality:
<?php function voldemortensen_reset_postpass_cookie_name($cookie) { return 'wp-postpass_' . COOKIEHASH; } add_filter('post_password_cookie', 'voldemortensen_reset_postpass_cookie_name', 10, 3);
This ticket was mentioned in Slack in #core by voldemortensen. View the logs.
8 years ago
#28
@
8 years ago
- Keywords 4.8-early added
- Milestone changed from 4.7 to Future Release
While I still think we should change this behavior at some point, going to punt this, as we would have wanted to do that pre-beta. If somebody would like to open another ticket for reviewing the copy around post passwords, that would be lovely, though I suppose it's been this long so it may not be worth changing for 4.7.
#29
@
8 years ago
I'm not going to play the change milestone game, that's just immature, but I would like to voice my opinion that this is a relatively small change. We are currently making much larger and most likely more impactful changes in the middle of beta. I don't see reason why this shouldn't go in.
#30
@
8 years ago
It is too late to introduce a breaking change, especially behavior that goes back many many releases. On top of that, nobody else has picked up on this ticket and I did not and do not personally have bandwidth to usher it in for 4.7.
This ticket was mentioned in Slack in #core by flixos90. View the logs.
8 years ago
#33
@
8 years ago
- Keywords early added; 4.8-early removed
- Milestone changed from 4.8 to Future Release
#35
@
8 years ago
You asked for a use-case where the actual behaviour makes sense? ;-) We got a customer, that uses same passwords over several Pages to make a complete "Chapter" available for the Users without having to "log in" again on every page.
I'm pretty unsure if you can call the actual behaviour a real "security risk". If the User just has to enter a password, he ALREADY KNOWS for the Site, i would not call that "secure" actually.
Let's be honest: 2 Parts of the actual Solution make it practically impossible to make it secure. One per-Page-Password will never be really secure. And the Password hashed in the Cookie? Will always be unsecure in my point of view.
The One-Cookie-Per-Post-Solution implies Problems for all Sites, that rely on CDN-Services like Cloudfront, that rely on Cookies and/or Header-Information to cache content. Having individual Cookies for every post will make it impossible to to whitelist Cookies properly AND preventing that a logged-in will get a complete personal cache because of a very specific Cookie-Set.
For preventing the above behaviour even for the normal password-cookie, i set the lifetime of the password-cookie to 1 second. This makes the user to enter the password on every page. Even if you reload the actual one.
<?php function szs_set_cookie_expire () { return time() + 1; // 1 second. } add_filter('post_password_expires', 'szs_set_cookie_expire', 99);
But by that way no password-cookie ist sent at the next request and a CDN can response with the cached version as expected.
I would imagine that making the underlying security-problems with using the Post-Passwords more transparent is more important than changing one unsecure behaviour to a practically just as unsecure behaviour.
This ticket was mentioned in Slack in #core by melchoyce. View the logs.
7 years ago
#37
@
7 years ago
- Milestone changed from 4.9 to Future Release
Punting because early
and we're in 4.9-beta1.
#41
@
3 months ago
- Milestone Future Release deleted
- Resolution set to wontfix
- Status changed from new to closed
Making an executive decision to close this due to little interest for the best part of a decade.
Notes:
- The support guide for protecting posts with a password documents the password protection feature well, including its behaviour when multiple posts share the same password.
- The
post_password_required
filter has been in place since 4.7 and allows custom authentication checks for post passwords (see #38056).
Make passwords post-specific