Make WordPress Core

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: wp_kc's profile wp_kc 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 @mukesh27
5 years ago

  • Component changed from Administration to General
  • Focuses administration coding-standards removed
  • Keywords dev-feedback added; needs-design-feedback removed

#2 @sabernhardt
4 years ago

  • Version trunk deleted

Related: #16858

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 @ootwch
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]

#5 @ootwch
4 years ago

Updated PR for latest trunk.

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.php wp_calculate_image_srcset() — domain-identity check (not URL construction)
  • wp-login.php RELOCATE block — deliberately reads the actual host to update siteurl

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:

https://github.com/WordPress/wordpress-develop/blob/eef6fb283068cdcc8833156e16c27aa89c14f94c/src/wp-includes/link-template.php#L3799

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:

https://github.com/WordPress/wordpress-develop/blob/eef6fb283068cdcc8833156e16c27aa89c14f94c/src/wp-includes/link-template.php#L3799

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?

Note: See TracTickets for help on using tickets.