#16449 closed defect (bug) (fixed)
incorrect referer check in check_admin_referer()
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 3.1 | Priority: | normal |
Severity: | normal | Version: | 3.0.4 |
Component: | Security | Keywords: | has-patch needs-testing |
Focuses: | Cc: |
Description
The check_admin_referer() function is defined this way:
"Tests if the current request was referred from an admin page, or (given $action parameter) if the current request carries a valid nonce. Used to avoid security exploits."
The older, less secure form with no parameter ("check_admin_referer()") still works, it's not documented as deprecated anywhere, and at least one plugin with more than 190,000 downloads uses it.
The problem is that it's not secure. An attacker can fool it easily like this:
1) Put up pages "one.html" and "two.html" on evilsite.com, where "one.html" includes "two.html" in an iframe, and "two.html" performs a CSRF attack against the admin part of a plugin in Victim's WP install.
2) "one.html" should include Victim's WP admin URL in the query string for the URL given for "two.html" in the iframe ( http://evilsite.com/two.html?foobar=http://victim.vi/wp-admin/ ). The Referer check in check_admin_referer() erroneously uses strpos() !== false, so it will be fooled by a Referer that looks like this.
3) Entice a logged-in administrator to visit http://evilsite.com/one.html .
I have attached a patch that should correct this issue by changing the strpos() call. I hope it won't break anything..
Ulf Harnhammar
Attachments (3)
Change History (11)
#1
@
14 years ago
Good catch. In the future, please send security-related items to security / wordpress / org.
The older, less secure form with no parameter ("check_admin_referer()") still works, it's not documented as deprecated anywhere
We should throw a deprecated notice if it is used without a parameter.
To be clear, this is not an issue in core, as no active code uses the function without a parameter. But it would be a security bonus to plugins that haven't been updated to use nonces.
#2
@
14 years ago
- Milestone changed from Awaiting Review to 3.1
Probably should fix this in 3.1 (and 3.0). The patch looks good.
#4
@
14 years ago
- Owner set to markjaquith
- Status changed from new to accepted
Proposal: Fix it for 3.1 and 3.0. Deprecate calling it with no first parameter in 3.2.
patch