#2858 closed defect (bug) (fixed)
Problem with wp_get_referer()
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 2.0.4 |
Component: | Administration | Keywords: | |
Focuses: | Cc: |
Description
It seems that because of new wp_get_referer() function I get redirected incorrectly in the admin "Options". When I save options, instead of going back to the same option page (like "General Options") it redirects back to the previous page.
- Go to "General Options"
- Go to "Writing Options"
- Click "Update Options >>"
- Redirected to "General Options", not the "Writing Options" as expected
Version I use is latest SVN of 2.0 branch.
Change History (17)
#2
@
17 years ago
Actually it is even worse, check wp_referer_field() function:
$ref = ( false === wp_get_referer() ) ? $_SERVER['REQUEST_URI'] : wp_get_referer();
it will almost always output the return of wp_get_referer(), which in turn will be referer to the page that made the request. So even with my proposed fix it will redirect to the previous page if $_SERVERHTTP_REFERER? is not available (and will ignore $_SERVERREQUEST_URI?).
It is very confusing actually: the field _wp_http_referer should contain the referer, and probably wp_get_referer() should used some other first, which should contain $_SERVERREQUEST_URI? (since our goal is to get page URI, not the URI that referred to it).
#3
@
17 years ago
O.K., after reading the purpose of wp_get_referer() it seems to me that there is some mix up - when received by script:
_wp_http_referer will contain the referer to the page that made the request, not the referer to the script itself
_wp_original_http_referer - will containt the referer to the script itself, not the original referer to the requesting page
So probably _wp_http_referer and _wp_original_http_referer should be swapped or renamed (to avoid future confusion).
#4
@
17 years ago
- Version set to 2.0.4
[3919] should help. We definitely need to make more clear which functions should set the current page as the referrer and which should pass along the referrer.
#7
@
17 years ago
Yeah, my bad. That original patch wasn't quite ready for prime time. :-)
The intention was for _wp_http_referer field to contain the URL of the current page (the referer, to the page the form submits to). _wp_original_http_referer is supposed to be used for cases where you do something like this:
- Start at A
- Go to B
- Do foo at B
- Do bar at B
- return to A
i.e. you're not going back one step, but back to a original step. So that's the one that should be passed along, and _wp_http_referer should always be the previous page.
#8
@
17 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
Can someone confirm that these functions don't open us up to HTTP response splitting attacks?
#10
@
17 years ago
Nice call, Matt. For safety, we should strip out "\r\n" from $_REQUEST_wp_http_referer? just to be slightly paranoid. A link with _wp_http_referer with \r\n in it could be used to do an HTTP response splitting attack (although most likely anything that would refer is likely to be nonce protected anyways, but I haven't looked too deeply into the code to confirm that, and my bet is that the information would be used to refer backwards if a user was to hit "No", so yes, if my guess is correct (I'll dive into the code and check later), this does open us up. A simple return str_replace("\r\n", '', $ref)
on line 864 of functions.php would solve the issue.
#12
@
17 years ago
Note that wp_redirect already strip \r and \n. I think we also need to strip %0d and %0a.
Should we use wp_redirect() everywhere?
#13
@
17 years ago
Our approach in another part of the code is a whitelist of characters, which I believe is the safest approach as we never know what sort of weird unicode character + browser bug could make this an issue in the future.
This is the code we have in wp-login, I assume it is a comprehensive regex and I see no reason we shouldn't apply it to all places we send Location headers:
$redirect_to = preg_replace('|[^a-z0-9-~+_.?#=&;,/:]|i', '', $_REQUEST['redirect_to']);
#15
@
17 years ago
The only meta characters in a character class are : \ - and ]
I think that regex is safe.
As I look to the wp_get_referer() function, it looks like an obvious defect. It shouldn't be:
because if you will update several option pages one after another, _wp_http_referer will be kept the same, even if $_SERVERHTTP_REFERER? is valid and present. It should be:
In this case new referer will be preferred (if it exists).