From badd58ada645cacc184f6fc927e26550a446d143 Mon Sep 17 00:00:00 2001 From: Keith Solomon Date: Sun, 26 Apr 2026 14:42:11 -0500 Subject: [PATCH] fix: prevent unsafe url rewrites --- src/Url/UrlTransformer.php | 45 +++++++++++++++++--- tests/Unit/Url/UrlTransformerTest.php | 61 +++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 6 deletions(-) diff --git a/src/Url/UrlTransformer.php b/src/Url/UrlTransformer.php index 99db928..23e77ce 100644 --- a/src/Url/UrlTransformer.php +++ b/src/Url/UrlTransformer.php @@ -4,26 +4,59 @@ namespace WPContentSync\Url; final class UrlTransformer { public function transformString( string $value, UrlMappingCollection $mappings ): string { - $transformed = $value; + $replacements = array(); foreach ( $mappings->all() as $mapping ) { - $transformed = $this->replaceMapping( $transformed, $mapping ); + $replacements = array_merge( $replacements, $this->replacementsForMapping( $mapping ) ); } - return $transformed; + if ( array() === $replacements ) { + return $value; + } + + return preg_replace_callback( + $this->replacementPattern( array_keys( $replacements ) ), + static function ( array $matches ) use ( $replacements ): string { + return $replacements[ $matches[0] ]; + }, + $value + ) ?? $value; } - private function replaceMapping( string $value, UrlMapping $mapping ): string { + /** + * @return array + */ + private function replacementsForMapping( UrlMapping $mapping ): array { $source = $mapping->sourceUrl(); $destination = $mapping->destinationUrl(); - $replacements = array( + return array( $source => $destination, str_replace( '&', '&', $source ) => str_replace( '&', '&', $destination ), $this->toProtocolRelative( $source ) => $this->toProtocolRelative( $destination ), + str_replace( '&', '&', $this->toProtocolRelative( $source ) ) => str_replace( '&', '&', $this->toProtocolRelative( $destination ) ), + ); + } + + /** + * @param array $sources Replacement sources. + */ + private function replacementPattern( array $sources ): string { + usort( + $sources, + static function ( string $left, string $right ): int { + return strlen( $right ) <=> strlen( $left ); + } ); - return strtr( $value, $replacements ); + $quoted = array_map( + static function ( string $source ): string { + return preg_quote( $source, '~' ); + }, + $sources + ); + + return '~(?transformString( 'https://example.test https://cdn.example.test/image.jpg', $mappings ) ); } + + public function test_it_does_not_cascade_mapping_destinations_into_other_sources(): void { + $transformer = new UrlTransformer(); + $mappings = new UrlMappingCollection( + array( + new UrlMapping( 'https://a.example.test', 'https://b.example.test' ), + new UrlMapping( 'https://b.example.test', 'https://c.example.test' ), + ) + ); + + self::assertSame( + 'https://b.example.test/page https://c.example.test/page', + $transformer->transformString( 'https://a.example.test/page https://b.example.test/page', $mappings ) + ); + } + + public function test_it_does_not_rewrite_partial_host_matches(): void { + $transformer = new UrlTransformer(); + $mappings = new UrlMappingCollection( + array( + new UrlMapping( 'https://example.test', 'https://staging.example.test' ), + ) + ); + + self::assertSame( + 'https://example.test.evil/path https://staging.example.test/path', + $transformer->transformString( + 'https://example.test.evil/path https://example.test/path', + $mappings + ) + ); + } + + public function test_it_prefers_more_specific_overlapping_mappings(): void { + $transformer = new UrlTransformer(); + $mappings = new UrlMappingCollection( + array( + new UrlMapping( 'https://example.test', 'https://staging.example.test' ), + new UrlMapping( 'https://example.test/uploads', 'https://media.staging.example.test/uploads' ), + ) + ); + + self::assertSame( + 'https://media.staging.example.test/uploads/image.jpg', + $transformer->transformString( 'https://example.test/uploads/image.jpg', $mappings ) + ); + } + + public function test_it_rewrites_escaped_protocol_relative_urls(): void { + $transformer = new UrlTransformer(); + $mappings = new UrlMappingCollection( + array( + new UrlMapping( 'https://example.test/path?a=1&b=2', 'https://staging.example.test/path?a=1&b=2' ), + ) + ); + + self::assertSame( + '//staging.example.test/path?a=1&b=2', + $transformer->transformString( '//example.test/path?a=1&b=2', $mappings ) + ); + } }