Skip to content

Commit

Permalink
Harden origin checking
Browse files Browse the repository at this point in the history
  • Loading branch information
westonruter committed Nov 8, 2024
1 parent 7b5a1ab commit 5f64507
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 1 deletion.
31 changes: 30 additions & 1 deletion plugins/optimization-detective/storage/rest-api.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,35 @@ function od_register_endpoint(): void {
}
add_action( 'rest_api_init', 'od_register_endpoint' );

/**
* Determines if the HTTP origin is an authorized one.
*
* Note that `is_allowed_http_origin()` is not used directly because the underlying `get_allowed_http_origins()` does
* not account for the URL port (although there is a to-do comment committed in core to address this). Additionally,
* the `is_allowed_http_origin()` function in core for some reason returns a string rather than a boolean.
*
* @since n.e.x.t
*
* @see get_allowed_http_origins()
* @see is_allowed_http_origin()
*
* @param string $origin Origin to check.
* @return bool Whether the origin is allowed.
*/
function od_is_allowed_http_origin( string $origin ): bool {
$allowed_origins = get_allowed_http_origins();
$home_url_port = wp_parse_url( home_url(), PHP_URL_PORT );
if ( is_int( $home_url_port ) ) {
$allowed_origins = array_map(
static function ( string $allowed_origin ) use ( $home_url_port ): string {
return $allowed_origin . ':' . (string) $home_url_port;
},
$allowed_origins
);
}
return in_array( $origin, $allowed_origins, true );
}

/**
* Handles REST API request to store metrics.
*
Expand All @@ -102,7 +131,7 @@ function od_register_endpoint(): void {
function od_handle_rest_request( WP_REST_Request $request ) {
// Block cross-origin storage requests since by definition URL Metrics data can only be sourced from the frontend of the site.
$origin = $request->get_header( 'origin' );
if ( null === $origin || home_url() !== $origin ) {
if ( null === $origin || ! od_is_allowed_http_origin( $origin ) ) {
return new WP_Error(
'rest_cross_origin_forbidden',
__( 'Cross-origin requests are not allowed for this endpoint.', 'optimization-detective' ),
Expand Down
21 changes: 21 additions & 0 deletions plugins/optimization-detective/tests/storage/test-rest-api.php
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ public function test_rest_request_bad_params( array $params ): void {
*
* @covers ::od_register_endpoint
* @covers ::od_handle_rest_request
* @covers ::od_is_allowed_http_origin
*/
public function test_rest_request_without_origin(): void {
$request = new WP_REST_Request( 'POST', self::ROUTE );
Expand All @@ -260,6 +261,7 @@ public function test_rest_request_without_origin(): void {
*
* @covers ::od_register_endpoint
* @covers ::od_handle_rest_request
* @covers ::od_is_allowed_http_origin
*/
public function test_rest_request_cross_origin(): void {
$request = new WP_REST_Request( 'POST', self::ROUTE );
Expand All @@ -271,6 +273,25 @@ public function test_rest_request_cross_origin(): void {
$this->assertSame( 0, did_action( 'od_url_metric_stored' ) );
}

/**
* Test REST API request when 'home_url' is filtered.
*
* @covers ::od_register_endpoint
* @covers ::od_handle_rest_request
* @covers ::od_is_allowed_http_origin
*/
public function test_rest_request_origin_when_home_url_filtered(): void {
$request = $this->create_request( $this->get_valid_params() );
add_filter(
'home_url',
static function ( string $url ): string {
return trailingslashit( $url ) . 'home/en/?foo=bar#baz';
}
);
$response = rest_get_server()->dispatch( $request );
$this->assertSame( 200, $response->get_status() );
}

/**
* Test not sending JSON data.
*
Expand Down

0 comments on commit 5f64507

Please sign in to comment.