Make WordPress Core

Opened 3 years ago

Last modified 20 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 (5)

#1 @mukesh27
3 years ago

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

#2 @sabernhardt
2 years ago

  • Version trunk deleted

Related: #16858

This ticket was mentioned in PR #2104 on WordPress/wordpress-develop by ootwch.


2 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
2 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
20 months ago

Updated PR for latest trunk.

Note: See TracTickets for help on using tickets.