#14556 closed enhancement (fixed)
get_pagenum_link() needs esc_url()
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 3.4 | Priority: | normal |
Severity: | normal | Version: | 3.0.1 |
Component: | Security | Keywords: | has-patch |
Focuses: | Cc: |
Description
We're using get_pagenum_link() to build a page navigation instead of older/newer posts only. We've found this vulnerability on multiple sites, here's an example
http://robertbasic.com/blog/?%3E%22'%3E%3CScRiPt%3Ealert(428017202033)%3C/ScRiPt%3E
Attachments (2)
Change History (20)
#3
@
13 years ago
- Severity changed from critical to normal
You need to use esc_url() before outputing what get_pagenum_link() returns.
Decreasing severity because it's used properly everywhere in core.
Should probably add a warning somewhere in the function's doc.
#5
follow-up:
↓ 7
@
13 years ago
The code is here - http://robertbasic.com/blog/wordpress-paging-navigation/
If you're not using mod_rewrite, wouldn't esc_url() mess with navigation ?
#6
@
13 years ago
In future the correct way to report potential security issues is detailed here: http://codex.wordpress.org/Security_FAQ
#7
in reply to:
↑ 5
@
13 years ago
Replying to guigouz:
If you're not using mod_rewrite, wouldn't esc_url() mess with navigation ?
Nope. esc_url() is for escaping any type of URL.
#8
@
13 years ago
- Keywords reporter-feedback removed
- Summary changed from get_pagenum_link() vulnerable to XSS attacks to get_pagenum_link() needs esc_url()
#9
@
13 years ago
- Milestone Awaiting Review deleted
- Resolution set to wontfix
- Status changed from new to closed
Unless this gets fixed at a lower level, i.e. #13051, any functions that return links will return them unescaped. Only functions that return HTML with a link should have the link escaped.
#10
@
13 years ago
I ran into this same issue with my pagination plugin, WP-Paginate.
It doesn't seem to be an issue with get_comments_pagenum_link(), but unless I wrap get_pagenum_link() with esc_url(), I am able to create an XSS vulnerability.
I can see how it might be a complicated issue, but I would expect WordPress to sanitize values returned from it's functions, or at the very least provide a huge warning to theme/plugin developers of potential issues with certain functions.
#11
@
13 years ago
- Cc eric@… added
- Resolution wontfix deleted
- Status changed from closed to reopened
Given that #13051 refers to links that come from the database/options, I don't think this issue is the same.
The problem is that someone can pass a query string that when used in conjunction with get_pagenum_link() can create a XSS vulnerability.
In the get_pagenum_link function in wp-includes/link-template.php, what about changing:
$result = $base . $request . $query_string;
to:
$result = esc_url($base . $request . $query_string);
or:
$result = $base . $request . htmlspecialchars($query_string);
I will send an email to security@…, but wanted to re-open this issue for consideration.
#13
@
12 years ago
Attached a couple of patches that patch get_pagenum_link()
(the initial function) and the functions that use it using esc_url()
. IMO it'd be best to patch the former since calls don't necessarily use the latter functions.
#15
@
11 years ago
- Milestone changed from Awaiting Review to 3.4
- Type changed from defect (bug) to enhancement
I'm not able to reproduce. Please paste the code you're using to generate the page links.