Opened 5 years ago
Last modified 2 months ago
#53998 new enhancement
Use network_home_url() instead of $_SERVER['HTTP_HOST'] for added safety.
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Milestone: | Awaiting Review | Priority: | normal |
| Severity: | normal | Version: | |
| Component: | General | Keywords: | dev-feedback has-patch |
| Focuses: | Cc: |
Description
Would it not be safer from XSS if uses of $_SERVER[HTTP_HOST] were replaced with network_home_url()? It looks to me like network_home_url() reads the server host name from the site settings instead of relying on a possibly manipulated $_SERVER[HTTP_HOST] value.
For example, I came across this code in /wp-admin/includes/class-wp-list-table.php...
<?php /** * Displays the pagination. * * @since 3.1.0 * * @param string $which */ protected function pagination( $which ) { ... $current_url = set_url_scheme( 'http://' . $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI'] ); ... }
Wouldn't this be safer if it were re-written as...
<?php /** * Displays the pagination. * * @since 3.1.0 * * @param string $which */ protected function pagination( $which ) { ... $current_url = network_home_url( $_SERVER['REQUEST_URI'] ); ... }
A search through the WP source code shows $_SERVER[HTTP_HOST] is used 27 times across 15 files.
Change History (9)
#1
@
5 years ago
- Component changed from Administration to General
- Focuses administration coding-standards removed
- Keywords dev-feedback added; needs-design-feedback removed
This ticket was mentioned in PR #2104 on WordPress/wordpress-develop by ootwch.
4 years ago
#3
- Keywords has-patch added
Replaced URL's based on $_SERVER[HTTP_HOST] with calls to network_home_url()
I cannot judge the security implications mentioned in the ticket, but I believe it is just a bug to not use the home/site url settings here. Fixing this makes it significantly simpler to run wordpress behind a reverse proxy, so workarounds like https://stackoverflow.com/questions/18443507/wordpress-behind-reverse-proxy-admin-issues should not be needed anymore.
Unit tests npm run test:php seem to be passing.
Trac ticket: Use network_home_url() instead of $_SERVER['HTTP_HOST' for added safety]
#4
@
4 years ago
Added a PR for this (see comment to PR)
Note: .htaccess still has to be modified behind a reverse proxy, otherwise mod_dir will try to handle the directories and insist on messing up the URL's.
# add a trailing slash to directories, so that mod_dir does not produce an invalid redirect
# https://stackoverflow.com/questions/32130330/htaccess-rewriterule-redirects-to-wrong-url
RewriteCond %{DOCUMENT_ROOT}/\$1 -d
RewriteRule ^(.*?[^/])\$ %{HTTP:x-forwarded-prefix}%{REQUEST_URI}/ [L,R=302]
This ticket was mentioned in PR #10939 on WordPress/wordpress-develop by ootwch.
2 months ago
#6
Replace manual URL construction from $_SERVER['HTTP_HOST'] with network_home_url().
Several places in WordPress core build absolute URLs by concatenating raw $_SERVER['HTTP_HOST'] with $_SERVER['REQUEST_URI']:
$url = ( is_ssl() ? 'https://' : 'http://' ) . $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI'];
// or
$url = set_url_scheme( 'http://' . $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI'] );
This breaks when WordPress is behind a reverse proxy that rewrites paths — HTTP_HOST reflects the proxy's internal hostname and REQUEST_URI may have had a path prefix stripped. WordPress already provides network_home_url() which reads the canonical URL from the database and handles scheme, host, and path prefix correctly. This PR replaces every URL-construction use of HTTP_HOST with the appropriate WordPress URL function.
Files changed (11 files, net −24 lines):
| File | What changed |
|---|---|
wp-admin/includes/class-wp-list-table.php | 2× pagination / column-sort current URL |
wp-admin/includes/misc.php | wp_admin_canonical_url()
|
wp-includes/admin-bar.php | Customizer menu current URL |
wp-includes/blocks/loginout.php | Login/logout block redirect URL |
wp-includes/canonical.php | redirect_canonical() fallback URL
|
wp-includes/class-wp-recovery-mode.php | Recovery-mode redirect (also removes dead $scheme variable)
|
wp-includes/functions.php | wp_auth_check_html() domain comparison
|
wp-includes/general-template.php | wp_login_form() default redirect
|
wp-includes/nav-menu-template.php | Custom menu-item active-state matching |
wp-includes/pluggable.php | auth_redirect() — 3 SSL redirects + login redirect; also wp_redirect() → wp_safe_redirect()
|
wp-login.php | SSL force-redirect |
Intentionally unchanged HTTP_HOST usages:
wp-includes/media.phpwp_calculate_image_srcset()— domain-identity check (not URL construction)wp-login.phpRELOCATEblock — deliberately reads the actual host to updatesiteurl
Trac ticket: https://core.trac.wordpress.org/ticket/53998
## Use of AI Tools
This PR was developed with the assistance of Cursor IDE with Claude. The AI was used for:
- Verifying that all instances of
$_SERVER['HTTP_HOST']used for URL construction in core are considered - Analysing which instances are URL construction (replaced) vs. domain comparison/discovery (kept)
- Drafting the replacement code and resolving rebase conflicts from 6.8 → 6.9
- Generating the text of this request
All changes were reviewed, tested, and approved by the author. Most of these changes have already been done in 2022.
@westonruter commented on PR #10939:
2 months ago
#7
I don't believe this will work as intended. For example, the $_SERVER['REQUEST_URI'] includes the request's current full path. Nevertheless, network_home_url() will _also_ prepend a path for the network:
If the network doesn't exist at the root, then this would result in duplicated path segment.
ootwch commented on PR #10939:
2 months ago
#8
I don't believe this will work as intended. For example, the
$_SERVER['REQUEST_URI']includes the request's current full path. Nevertheless,network_home_url()will _also_ prepend a path for the network:
If the network doesn't exist at the root, then this would result in duplicated path segment.
Understood, I think. In my setup (CI dev environment, not production) that does not seem to happen because the reverse proxy corrects this.
Cursor recommends a dedicated utility function to fix this; I can adapt the CR and verify this in my own setup, but this is then more than than the "just fix a couple of lines" change I had originally intended.. and my tests will not be encompassing enough.
Still, I think the original issue still exists (and has existed for a long time); would be nice to get some improvements for this into core, perhaps.
/**
* Builds a full URL for the current request using the site's configured host.
*
* Replaces raw $_SERVER['HTTP_HOST'] with the host from home_url(), which
* respects the DB-configured site address. This is important behind reverse
* proxies or load balancers where HTTP_HOST may not match the public host.
*
* If $_SERVER['REQUEST_URI'] already contains the home path (standard setup),
* only the scheme and host are swapped. If the home path is missing from
* REQUEST_URI (e.g. a reverse proxy stripped a path prefix), the home path
* is prepended automatically.
*
* @since 6.9.0
*
* @return string Full URL of the current request.
*/
function wp_get_current_request_url() {
$home = home_url( '/' );
$home_path = wp_parse_url( $home, PHP_URL_PATH ) ?: '/';
if ( str_starts_with( $_SERVER['REQUEST_URI'], $home_path ) ) {
// Standard case: REQUEST_URI already includes the home path.
// Just replace scheme + host, keep REQUEST_URI as-is.
$parsed = wp_parse_url( $home );
$host = $parsed['host'] ?? $_SERVER['HTTP_HOST'];
if ( isset( $parsed['port'] ) ) {
$host .= ':' . $parsed['port'];
}
return set_url_scheme( 'http://' . $host . $_SERVER['REQUEST_URI'] );
}
// Reverse-proxy case: REQUEST_URI is missing the home path prefix.
// Let home_url() prepend it.
return home_url( $_SERVER['REQUEST_URI'] );
}
@westonruter commented on PR #10939:
2 months ago
#9
What about the get_self_link() function?
Related: #16858