Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/changelog/fix-blocklist-host-normalization
Original file line number Diff line number Diff line change
@@ -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.
51 changes: 45 additions & 6 deletions includes/class-moderation.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down Expand Up @@ -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 );
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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;
Comment on lines +345 to 349
}
Expand All @@ -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.
*
Expand Down Expand Up @@ -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 ) ),
);
Comment on lines +415 to 421
foreach ( $blocked_domains as $domain ) {
if ( \in_array( $domain, $urls, true ) ) {
Expand Down
140 changes: 140 additions & 0 deletions tests/phpunit/tests/includes/class-test-moderation.php
Original file line number Diff line number Diff line change
Expand Up @@ -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' );
}
}