Opened 13 years ago
Last modified 5 days ago
#5272 reopened defect (bug)
WordPress allows anonymous user to see slug for private post by guessing post number
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 5.7 | Priority: | normal |
Severity: | normal | Version: | 2.3.1 |
Component: | Security | Keywords: | has-patch has-unit-tests |
Focuses: | Cc: |
Description
I have pretty permalinks enabled, and I set a post as private.
Entering http://blog.url/?p=(postid) will redirect the user, any user, to http://blog.url/perma/link/, and only then give him a 404 error.
Depending on permalink structure, this shows the private post's title to anyone who figures out its post number.
Attachments (1)
Change History (26)
#2
@
13 years ago
- Keywords needs-patch added
- Summary changed from Wodpress allows anonymous user to see slug for private post by guessing post number to WordPress allows anonymous user to see slug for private post by guessing post number
Luckily, you can fix it. :)
#3
@
13 years ago
- Owner changed from anonymous to pishmishy
- Status changed from new to assigned
Confirming that the bug exists in trunk. Worth fixing as the slug may have been automatically derived from post data that the user didn't want to disclose.
#4
@
13 years ago
- Keywords has-patch canonical redirection private added; needs-patch removed
Fix is to add an additional condition to be satisfied before canonical redirection can take place.
Not entirely happy with my patch but it does fix the problem. Someone more familiar with query.php can probably come up with an is_private() function. is_empty works just as well though.
#5
@
13 years ago
Tempted to close this one too. I think it's important to disclose as little information as possible but the fix has been on offer and there doesn't appear to be much demand for it.
#6
@
13 years ago
Too bad - I can't see how this is not a major privacy issue.
If we ever want to see WordPress used as a CMS for a major corporation, we can't have their secret keynote surprise open for the world to see.
#7
@
12 years ago
- Keywords needs-patch added; has-patch canonical redirection private removed
The patch is invalid. There is some reprocessing of 404 code further down, that ends up not getting processed. Imo, what WP should do here is return a 403 error.
#8
@
12 years ago
- Milestone 2.9 deleted
- Resolution set to wontfix
- Status changed from assigned to closed
there's a dup of this one somewhere, and it shoud get wontfixed too.
#9
@
11 months ago
- Component changed from General to Security
- Keywords needs-unit-tests added
- Milestone set to Future Release
- Resolution wontfix deleted
- Severity changed from major to normal
- Status changed from closed to reopened
Reopening following discussion with @whyisjake.
This ticket was mentioned in PR #433 on WordPress/wordpress-develop by donmhico.
6 months ago
- Keywords has-patch added; needs-patch removed
Prevent canonical redirect for private post.
Trac ticket: https://core.trac.wordpress.org/ticket/5272
TODO:
- Unit test.
- Allow the redirect if user is logged and has permission to read the post.
#11
@
6 months ago
Added a patch above however it still need some work. Will try to update the PR soon.
#12
@
6 months ago
- Keywords needs-testing added; needs-unit-tests removed
Updated the PR with unit tests. Need more people to test :)
#13
@
6 months ago
- Milestone changed from Future Release to 5.6
- Owner changed from pishmishy to SergeyBiryukov
- Status changed from reopened to reviewing
This ticket was mentioned in PR #479 on WordPress/wordpress-develop by peterwilsoncc.
5 months ago
- Keywords has-unit-tests added
Trac ticket: https://core.trac.wordpress.org/ticket/5272
#16
@
5 months ago
peterwilsoncc commented on PR #433:
@donmhico
In parallel, I'd written some code elsewhere that I have added as PR #479 .
The code we'd written is similar so I'd make sure you get props.
There are few places in your patch that redirects are determined that had been missed, so I've expanded upon that to catch other places.
For the unit tests I expanded them to include both positive and negative use cases for all types of users, including the original author, you'll find them in the other PR.
If you think I have missed anything, please add comments.
Peter
#17
follow-up:
↓ 18
@
5 months ago
As the friendly bot points out, I've pushed a PR to https://github.com/WordPress/wordpress-develop/pull/479
@TimothyBlynJacobs I've included a check for get_post_status_object( $redirect_obj->post_status )->public
, do you think I should logic check for private
or internal
too?
@SergeyBiryukov I included some tests for comment feed requests but was getting some unexpected results with ?name=slug&feed=rss2
style requests that I think are unrelated to this ticket. Do you know if they work?
#18
in reply to:
↑ 17
@
5 months ago
Replying to peterwilsoncc:
@TimothyBlynJacobs I've included a check for
get_post_status_object( $redirect_obj->post_status )->public
, do you think I should logic check forprivate
orinternal
too?
I really don't know. I don't think it's valid for more than one of public
, protected
or private
to be set to true
. But since the function doesn't enforce that perhaps some developers have accidentally done that?
I guess the publicly_queryable
post status arg might also be the thing to check? At a first glance, I can't find it being used anywhere in Core though.
#19
@
3 months ago
In conversation with @dd32:
I’d personally lean towards get_post_status_object( $status )->public as the filterable way, but private isn’t a public post status.
This ticket was mentioned in PR #745 on WordPress/wordpress-develop by peterwilsoncc.
2 months ago
https://core.trac.wordpress.org/ticket/5272
Starting with some unit tests -- they fail.
#23
@
2 months ago
- Milestone changed from 5.6 to Future Release
- Resolution fixed deleted
- Status changed from closed to reopened
This ticket was mentioned in PR #863 on WordPress/wordpress-develop by peterwilsoncc.
12 days ago
https://core.trac.wordpress.org/ticket/5272 but better this time.
Oh, and on decent hours I even know how to spell WordPress :(