Skip to content

Commit

Permalink
Merge pull request #1077 from Automattic/fix/font-url-protocol
Browse files Browse the repository at this point in the history 
Fix handling of font stylesheets with non-HTTPS scheme or scheme-less URLs
  • Loading branch information
@westonruter
westonruter authored Apr 17, 2018
2 parents c15a368 + 7e5789e commit 5754263
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 24 deletions.
42 changes: 32 additions & 10 deletions includes/sanitizers/class-amp-style-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,22 +311,39 @@ public function get_validated_url_file_path( $url, $allowed_extensions = array()
$url = $this->base_url . $url;
}

// Strip query and fragment from URL.
$url = preg_replace( ':[\?#].*$:', '', $url );
$remove_url_scheme = function( $schemed_url ) {
return preg_replace( '#^\w+:(?=//)#', '', $schemed_url );
};

// Strip URL scheme, query, and fragment.
$url = $remove_url_scheme( preg_replace( ':[\?#].*$:', '', $url ) );

$includes_url = $remove_url_scheme( includes_url( '/' ) );
$content_url = $remove_url_scheme( content_url( '/' ) );
$admin_url = $remove_url_scheme( get_admin_url( null, '/' ) );

$allowed_hosts = array(
wp_parse_url( $includes_url, PHP_URL_HOST ),
wp_parse_url( $content_url, PHP_URL_HOST ),
wp_parse_url( $admin_url, PHP_URL_HOST ),
);

$url_host = wp_parse_url( $url, PHP_URL_HOST );
if ( ! in_array( $url_host, $allowed_hosts, true ) ) {
/* translators: %s is the file URL */
return new WP_Error( 'disallowed_external_file_url', sprintf( __( 'Skipped file which does not have a recognized local host (%s).', 'amp' ), $url_host ) );
}

// Validate file extensions.
if ( ! empty( $allowed_extensions ) ) {
$pattern = sprintf( '/\.(%s)$/i', implode( '|', $allowed_extensions ) );
if ( ! preg_match( $pattern, $url ) ) {
/* translators: %s is the file URL */
return new WP_Error( 'amp_disallowed_file_extension', sprintf( __( 'Skipped file which does not have an allowed file extension (%s).', 'amp' ), $url ) );
return new WP_Error( 'disallowed_file_extension', sprintf( __( 'Skipped file which does not have an allowed file extension (%s).', 'amp' ), $url ) );
}
}

$includes_url = includes_url( '/' );
$content_url = content_url( '/' );
$admin_url = get_admin_url( null, '/' );
$file_path = null;
$file_path = null;
if ( 0 === strpos( $url, $content_url ) ) {
$file_path = WP_CONTENT_DIR . substr( $url, strlen( $content_url ) - 1 );
} elseif ( 0 === strpos( $url, $includes_url ) ) {
Expand All @@ -337,7 +354,7 @@ public function get_validated_url_file_path( $url, $allowed_extensions = array()

if ( ! $file_path || false !== strpos( '../', $file_path ) || 0 !== validate_file( $file_path ) || ! file_exists( $file_path ) ) {
/* translators: %s is file URL */
return new WP_Error( 'amp_file_path_not_found', sprintf( __( 'Unable to locate filesystem path for %s.', 'amp' ), $url ) );
return new WP_Error( 'file_path_not_found', sprintf( __( 'Unable to locate filesystem path for %s.', 'amp' ), $url ) );
}

return $file_path;
Expand Down Expand Up @@ -394,14 +411,19 @@ private function process_style_element( DOMElement $element ) {
private function process_link_element( DOMElement $element ) {
$href = $element->getAttribute( 'href' );

// Allow font URLs.
if ( $this->allowed_font_src_regex && preg_match( $this->allowed_font_src_regex, $href ) ) {
// Allow font URLs, including protocol-less URLs and recognized URLs that use HTTP instead of HTTPS.
$normalized_font_href = preg_replace( '#^(http:)?(?=//)#', 'https:', $href );
if ( $this->allowed_font_src_regex && preg_match( $this->allowed_font_src_regex, $normalized_font_href ) ) {
if ( $href !== $normalized_font_href ) {
$element->setAttribute( 'href', $normalized_font_href );
}
return;
}

$css_file_path = $this->get_validated_url_file_path( $href, array( 'css', 'less', 'scss', 'sass' ) );
if ( is_wp_error( $css_file_path ) ) {
$this->remove_invalid_child( $element, array(
'code' => $css_file_path->get_error_code(),
'message' => $css_file_path->get_error_message(),
) );
return;
Expand Down
66 changes: 52 additions & 14 deletions tests/test-amp-style-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -536,15 +536,27 @@ public function get_stylesheet_urls() {
admin_url( 'css/common.css' ),
ABSPATH . 'wp-admin/css/common.css',
),
'admin_with_host_https' => array(
set_url_scheme( admin_url( 'css/common.css' ), 'https' ),
ABSPATH . 'wp-admin/css/common.css',
),
'admin_with_host_http' => array(
set_url_scheme( admin_url( 'css/common.css' ), 'http' ),
ABSPATH . 'wp-admin/css/common.css',
),
'admin_with_no_host_scheme' => array(
preg_replace( '#^\w+:(?=//)#', '', admin_url( 'css/common.css' ) ),
ABSPATH . 'wp-admin/css/common.css',
),
'amp_disallowed_file_extension' => array(
content_url( 'themes/twentyseventeen/index.php' ),
null,
'amp_disallowed_file_extension',
'disallowed_file_extension',
),
'amp_file_path_not_found' => array(
content_url( 'themes/twentyseventeen/404.css' ),
null,
'amp_file_path_not_found',
'file_path_not_found',
),
);
}
Expand Down Expand Up @@ -581,23 +593,39 @@ public function get_font_urls() {
return array(
'tangerine' => array(
'https://fonts.googleapis.com/css?family=Tangerine',
true,
array(),
),
'tangerine2' => array(
'//fonts.googleapis.com/css?family=Tangerine',
array(),
),
'tangerine3' => array(
'http://fonts.googleapis.com/css?family=Tangerine',
array(),
),
'typekit' => array(
'https://use.typekit.net/abc.css',
true,
array(),
),
'fontscom' => array(
'https://fast.fonts.net/abc.css',
true,
array(),
),
'fontawesome' => array(
'https://maxcdn.bootstrapcdn.com/font-awesome/123/css/font-awesome.min.css',
true,
array(),
),
'fontbad' => array(
'bad_host' => array(
'https://bad.example.com/font.css',
false,
array( 'disallowed_external_file_url' ),
),
'bad_ext' => array(
home_url( '/bad.php' ),
array( 'disallowed_file_extension' ),
),
'bad_file' => array(
home_url( '/bad.css' ),
array( 'file_path_not_found' ),
),
);
}
Expand All @@ -606,21 +634,31 @@ public function get_font_urls() {
* Tests that font URLs get validated.
*
* @dataProvider get_font_urls
* @param string $url Font URL.
* @param bool $pass Whether the font URL is ok.
* @param string $url Font URL.
* @param array $error_codes Error codes.
*/
public function test_font_urls( $url, $pass ) {
public function test_font_urls( $url, $error_codes ) {
$dom = AMP_DOM_Utils::get_dom( sprintf( '<html><head><link rel="stylesheet" href="%s"></head></html>', $url ) ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedStylesheet

$validation_errors = array();

$sanitizer = new AMP_Style_Sanitizer( $dom, array(
'use_document_element' => true,
'use_document_element' => true,
'validation_error_callback' => function( $error ) use ( &$validation_errors ) {
$validation_errors[] = $error;
},
) );
$sanitizer->sanitize();

$this->assertEqualSets( $error_codes, wp_list_pluck( $validation_errors, 'code' ) );

$link = $dom->getElementsByTagName( 'link' )->item( 0 );
if ( $pass ) {
if ( empty( $error_codes ) ) {
$this->assertInstanceOf( 'DOMElement', $link );
$this->assertEquals( $url, $link->getAttribute( 'href' ) );
$this->assertEquals(
preg_replace( '#^(http:)?(?=//)#', 'https:', $url ),
$link->getAttribute( 'href' )
);
} else {
$this->assertEmpty( $link );
}
Expand Down

0 comments on commit 5754263

Please sign in to comment.