WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 5 years ago

#5487 closed defect (bug) (fixed)

query.php mistakenly uses is_admin() to check for admin privileges

Reported by: pishmishy Owned by: pishmishy
Milestone: 2.3.2 Priority: high
Severity: major Version: 2.3.1
Component: Security Keywords: query is_admin has-patch dev-feedback
Focuses: Cc:

Description (last modified by lloydbudd)

  1. Create a draft post
  2. Log out
  3. Visit http://yourblog.com/index.php/wp-admin/
    • is_admin() spots the wp-admin in the request and returns true
    • query.php uses is_admin() to decide to return future, draft or pending posts
  4. Future, draft and pending posts are displayed.

This doesn't require the ' in the request string as reported on Bugtraq.

See http://www.securityfocus.com/archive/1/485252/30/0/threaded

12/22 additional disclosure, with trivial, popular example: http://www.blackhatdomainer.com/how-to-know-today-what-shoemoney-is-going-to-post-tomorrow/

Attachments (2)

5487.patch (738 bytes) - added by pishmishy 6 years ago.
New patch improves is_admin()
5487.002.diff (429 bytes) - added by markjaquith 6 years ago.
Don't we need to fix this too?

Download all attachments as: .zip

Change History (20)

comment:1 pishmishy6 years ago

  • Status changed from new to assigned

comment:2 pishmishy6 years ago

See line 1172 of query.php for the misuse of is_admin()

comment:3 pishmishy6 years ago

  • Keywords has-patch dev-feedback added

Attached patch replaces is_admin() check with current_user_can('level_10'). Perhaps we could explicitly check the user's capabilities but I wasn't sure from the documentation which capabilities we should be looking at. Instead I've just checked if the user is the administrator or not.

comment:4 follow-up: docwhat6 years ago

What I did (Wordpress 2.3.1):

  • Logged into wp-admin with Firefox.
  • Created a new post called "DRAFT", with text "DRAFT"
  • I saved it (but did not publish it)
  • I opened another browser (Opera).
  • I tried using the URL you had above (modified for my site) and it does not show me drafts.
  • I tried adding the p=<post number> get argument, but I just get a blank page.

I cannot reproduce this problem.

Will the current_user_can() allow the author (possibly a non-admin) to view the draft post that he/she just wrote?

Ciao!

comment:5 ryan6 years ago

We do a current_user_can() check in the block of code already. is_admin() is used to see what context the user is in. Is the user in the admin? I think we need to retain is_admin() and have it check a constant set in admin.php to determine admin context.

comment:6 ryan6 years ago

Hmm, the current_user_can() check is just for private posts. I think we need both an is admin user and is in the admin checks.

comment:7 ryan6 years ago

Actually, edit-pages.php and edit.php filter the results of the is_admin() query. So I think all we need is a proper is_admin() check and not any cap checks.

pishmishy6 years ago

New patch improves is_admin()

comment:8 pishmishy6 years ago

New patch improves is_admin().
Old patch was confused over why is_admin() was used in the first place.
Thanks to Austin Matzko from wp-hackers for the idea.

comment:9 ryan6 years ago

Looks good to me.

comment:10 ryan6 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [6412]) Better is_admin() check from filosofo and pishmishy. fixes #5487

comment:11 in reply to: ↑ 4 docwhat6 years ago

Replying to docwhat:

What I did (Wordpress 2.3.1):

Finally, someone posted what was missing. The unposted drafts have a date of something really old (1969 or 1999). You have to search back into the archive to find it.

Ciao!

comment:12 ryan6 years ago

(In [6442]) Better is_admin() check from filosofo and pishmishy. fixes #5487 for 2.3

comment:13 lloydbudd6 years ago

  • Milestone changed from 2.4 to 2.3.2

comment:14 lloydbudd6 years ago

  • Description modified (diff)

markjaquith6 years ago

Don't we need to fix this too?

comment:15 markjaquith6 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

$wp_query->is_admin (the var) is checked in some places and is still using the old logic instead of the is_admin() function. Shouldn't we fix that too? See patch.

comment:16 ryan6 years ago

(In [6509]) Use is_admin. Props markjaquith. see #5487

comment:17 ryan6 years ago

(In [6510]) Use is_admin. Props markjaquith. see #5487

comment:18 ryan6 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.