diff --git a/.github/changelog/fix-blocklist-host-normalization b/.github/changelog/fix-blocklist-host-normalization new file mode 100644 index 000000000..44d447e9a --- /dev/null +++ b/.github/changelog/fix-blocklist-host-normalization @@ -0,0 +1,4 @@ +Significance: patch +Type: security + +Ensure blocked domains match regardless of letter case or a trailing dot, so they can no longer be bypassed. diff --git a/includes/class-moderation.php b/includes/class-moderation.php index b28164b01..2f02734a0 100644 --- a/includes/class-moderation.php +++ b/includes/class-moderation.php @@ -110,6 +110,10 @@ public static function activity_is_blocked_for_user( $activity, $user_id ) { * @return bool True on success, false on failure. */ public static function add_user_block( $user_id, $type, $value ) { + if ( self::TYPE_DOMAIN === $type ) { + $value = self::normalize_host( $value ); + } + switch ( $type ) { case self::TYPE_ACTOR: return Blocked_Actors::add( $user_id, $value ); @@ -146,6 +150,10 @@ public static function add_user_block( $user_id, $type, $value ) { * @return bool True on success, false on failure. */ public static function remove_user_block( $user_id, $type, $value ) { + if ( self::TYPE_DOMAIN === $type ) { + $value = self::normalize_host( $value ); + } + switch ( $type ) { case self::TYPE_ACTOR: return Blocked_Actors::remove( $user_id, $value ); @@ -196,6 +204,10 @@ public static function get_user_blocks( $user_id ) { * @return bool True on success, false on failure. */ public static function add_site_block( $type, $value ) { + if ( self::TYPE_DOMAIN === $type ) { + $value = self::normalize_host( $value ); + } + switch ( $type ) { case self::TYPE_ACTOR: // Site-wide actor blocking uses the BLOG_USER_ID. @@ -241,6 +253,10 @@ public static function add_site_blocks( $type, $values ) { return; } + if ( self::TYPE_DOMAIN === $type ) { + $values = \array_map( array( self::class, 'normalize_host' ), $values ); + } + foreach ( $values as $value ) { /** * Fired when a domain or keyword is blocked site-wide. @@ -263,6 +279,10 @@ public static function add_site_blocks( $type, $values ) { * @return bool True on success, false on failure. */ public static function remove_site_block( $type, $value ) { + if ( self::TYPE_DOMAIN === $type ) { + $value = self::normalize_host( $value ); + } + switch ( $type ) { case self::TYPE_ACTOR: // Site-wide actor unblocking uses the BLOG_USER_ID. @@ -322,8 +342,9 @@ public static function is_actor_blocked( $actor_uri, $user_id = 0 ) { return true; } - // Check site-wide domain blocks. - $actor_domain = \wp_parse_url( $actor_uri, PHP_URL_HOST ); + // Check site-wide domain blocks. Normalize the (attacker-controlled) host so a case + // variant or trailing dot cannot slip past a block; stored domains are normalized on insert. + $actor_domain = self::normalize_host( \wp_parse_url( $actor_uri, PHP_URL_HOST ) ); if ( $actor_domain && \in_array( $actor_domain, $site_blocks['domains'], true ) ) { return true; } @@ -344,6 +365,23 @@ public static function is_actor_blocked( $actor_uri, $user_id = 0 ) { return false; } + /** + * Normalize a host for blocklist comparison. + * + * DNS hosts are case-insensitive, a trailing dot denotes the same (absolute) host, and IPv6 + * literals may be bracketed, so lower-case, strip IPv6 brackets, and trim a trailing dot. + * Applied to incoming hosts before comparison and to domains on insert, keeping both sides + * canonical. + * + * @param string|null $host The host. + * @return string The normalized host. + */ + private static function normalize_host( $host ) { + $host = \strtolower( (string) $host ); + $host = \rtrim( $host, '.' ); + return \trim( $host, '[]' ); + } + /** * Check activity against blocklists. * @@ -374,11 +412,12 @@ private static function check_activity_against_blocks( $activity, $blocked_actor } } - // Check blocked domains. + // Check blocked domains. Normalize the (attacker-controlled) hosts so a case variant + // or trailing dot cannot slip past a block; stored domains are normalized on insert. $urls = array( - \wp_parse_url( $actor_id, PHP_URL_HOST ), - \wp_parse_url( $activity->get_id(), PHP_URL_HOST ), - \wp_parse_url( object_to_uri( $activity->get_object() ) ?? '', PHP_URL_HOST ), + self::normalize_host( \wp_parse_url( $actor_id, PHP_URL_HOST ) ), + self::normalize_host( \wp_parse_url( $activity->get_id(), PHP_URL_HOST ) ), + self::normalize_host( \wp_parse_url( object_to_uri( $activity->get_object() ) ?? '', PHP_URL_HOST ) ), ); foreach ( $blocked_domains as $domain ) { if ( \in_array( $domain, $urls, true ) ) { diff --git a/tests/phpunit/tests/includes/class-test-moderation.php b/tests/phpunit/tests/includes/class-test-moderation.php index c9ac2bcbd..7b98da92b 100644 --- a/tests/phpunit/tests/includes/class-test-moderation.php +++ b/tests/phpunit/tests/includes/class-test-moderation.php @@ -1075,4 +1075,144 @@ public function test_is_actor_blocked_mixed_scenarios() { Moderation::remove_site_block( 'domain', $domain ); Moderation::remove_user_block( $this->test_user_id, 'actor', $actor_uri3 ); } + + /** + * Domains are stored normalized so case and trailing-dot variants cannot bypass a block. + * + * @covers ::add_site_block + * @covers ::get_site_blocks + * @covers ::add_user_block + * @covers ::get_user_blocks + * @covers ::add_site_blocks + */ + public function test_domain_blocks_are_normalized_on_insert() { + Moderation::add_site_block( 'domain', 'Example.COM.' ); + $this->assertSame( array( 'example.com' ), Moderation::get_site_blocks()['domains'] ); + + Moderation::add_user_block( $this->test_user_id, 'domain', 'Spam.Example.COM' ); + $this->assertSame( array( 'spam.example.com' ), Moderation::get_user_blocks( $this->test_user_id )['domains'] ); + + Moderation::add_site_blocks( 'domain', array( 'Bulk.Example.COM.', 'OTHER.example.com' ) ); + $this->assertSame( + array( 'example.com', 'bulk.example.com', 'other.example.com' ), + Moderation::get_site_blocks()['domains'] + ); + } + + /** + * A normalized stored block can be removed regardless of the case/trailing dot supplied. + * + * @covers ::remove_site_block + * @covers ::remove_user_block + */ + public function test_domain_blocks_are_normalized_on_remove() { + Moderation::add_site_block( 'domain', 'example.com' ); + Moderation::remove_site_block( 'domain', 'Example.COM.' ); + $this->assertSame( array(), Moderation::get_site_blocks()['domains'] ); + + Moderation::add_user_block( $this->test_user_id, 'domain', 'spam.example.com' ); + Moderation::remove_user_block( $this->test_user_id, 'domain', 'Spam.Example.COM.' ); + $this->assertSame( array(), Moderation::get_user_blocks( $this->test_user_id )['domains'] ); + } + + /** + * Case and trailing-dot host variants in an actor URI cannot slip past a domain block. + * + * @covers ::is_actor_blocked + */ + public function test_is_actor_blocked_normalizes_host() { + Moderation::add_site_block( 'domain', 'blocked.example.com' ); + + $bypass_uris = array( + 'https://Blocked.Example.com/@user', // Upper-case host. + 'https://BLOCKED.EXAMPLE.COM/@user', // All caps. + 'https://blocked.example.com./@user', // Trailing FQDN dot. + 'https://Blocked.Example.COM./@user', // Mixed case + trailing dot. + ); + + foreach ( $bypass_uris as $uri ) { + $this->assertTrue( Moderation::is_actor_blocked( $uri ), "URI should be blocked: $uri" ); + } + + // A genuinely different domain still passes. + $this->assertFalse( Moderation::is_actor_blocked( 'https://blocked.example.org/@user' ) ); + + Moderation::remove_site_block( 'domain', 'blocked.example.com' ); + } + + /** + * Case and trailing-dot host variants in an activity cannot slip past a domain block. + * + * @covers ::activity_is_blocked + * @covers ::activity_is_blocked_site_wide + * @covers ::check_activity_against_blocks + */ + public function test_activity_blocking_normalizes_host() { + Moderation::add_site_block( 'domain', 'evil.example.com' ); + + $bypass_actors = array( + 'https://Evil.Example.com/@user', + 'https://EVIL.EXAMPLE.COM/@user', + 'https://evil.example.com./@user', + ); + + foreach ( $bypass_actors as $actor ) { + /* @var Activity $activity Activity. */ + $activity = Activity::init_from_array( + array( + 'type' => 'Create', + 'actor' => $actor, + 'object' => array( + 'id' => 'https://example.org/note/1', + 'type' => 'Note', + 'content' => 'Test', + ), + ) + ); + + $this->assertTrue( Moderation::activity_is_blocked( $activity ), "Actor should be blocked: $actor" ); + } + + Moderation::remove_site_block( 'domain', 'evil.example.com' ); + } + + /** + * A blocked domain is matched on the activity id and object URI hosts, not just the actor. + * + * @covers ::activity_is_blocked + * @covers ::check_activity_against_blocks + */ + public function test_activity_blocking_normalizes_id_and_object_hosts() { + Moderation::add_site_block( 'domain', 'evil.example.com' ); + + // Clean actor, but the activity id is on a blocked domain (case variant). + /* @var Activity $by_id Activity. */ + $by_id = Activity::init_from_array( + array( + 'id' => 'https://EVIL.example.com/activities/1', + 'type' => 'Create', + 'actor' => 'https://good.example.org/@user', + 'object' => array( + 'id' => 'https://good.example.org/note/1', + 'type' => 'Note', + 'content' => 'Test', + ), + ) + ); + $this->assertTrue( Moderation::activity_is_blocked( $by_id ), 'Blocked id host should block the activity.' ); + + // Clean actor and id, but the object URI is on a blocked domain (trailing dot). + /* @var Activity $by_object Activity. */ + $by_object = Activity::init_from_array( + array( + 'id' => 'https://good.example.org/activities/2', + 'type' => 'Announce', + 'actor' => 'https://good.example.org/@user', + 'object' => 'https://evil.example.com./note/2', + ) + ); + $this->assertTrue( Moderation::activity_is_blocked( $by_object ), 'Blocked object host should block the activity.' ); + + Moderation::remove_site_block( 'domain', 'evil.example.com' ); + } }