Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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' );
}
}