fix Secops issues (#411)

includes/class-mention.php

@@ -74,7 +74,7 @@ class Mention {
 	public static function replace_with_links( $result ) {
 		$metadata = get_remote_metadata_by_actor( $result[0] );
 
-		if ( ! is_wp_error( $metadata ) && ! empty( $metadata['url'] ) ) {
+		if ( ! empty( $metadata ) && ! is_wp_error( $metadata ) && ! empty( $metadata['url'] ) ) {
 			$username = ltrim( $result[0], '@' );
 			if ( ! empty( $metadata['name'] ) ) {
 				$username = $metadata['name'];

includes/class-shortcodes.php

@@ -114,7 +114,7 @@ class Shortcodes {
 
 				/** This filter is documented in wp-includes/post-template.php */
 				$excerpt = \apply_filters( 'the_content', $excerpt );
-				$excerpt = \str_replace( ']]>', ']]>', $excerpt );
+				$excerpt = \str_replace( ']]>', ']]>', $excerpt );
 			}
 		}
 

includes/class-signature.php

@@ -234,7 +234,7 @@ class Signature {
 	 *
 	 * @param string $key_id The URL to the public key.
 	 *
-	 * @return string The public key.
+	 * @return WP_Error|string The public key.
 	 */
 	public static function get_remote_key( $key_id ) { // phpcs:ignore
 		$actor = get_remote_metadata_by_actor( strip_fragment_from_url( $key_id ) ); // phpcs:ignore
@@ -244,7 +244,7 @@ class Signature {
 		if ( isset( $actor['publicKey']['publicKeyPem'] ) ) {
 			return \rtrim( $actor['publicKey']['publicKeyPem'] ); // phpcs:ignore
 		}
-		return null;
+		return new WP_Error( 'activitypub_no_remote_key_found', 'No Public-Key found' );
 	}
 
 	/**

includes/collection/class-followers.php

@@ -160,7 +160,7 @@ class Followers {
 	 * @param int    $user_id The ID of the WordPress User
 	 * @param string $actor   The Actor URL
 	 *
-	 * @return array|WP_Error The Follower (WP_Term array) or an WP_Error
+	 * @return array|WP_Error The Follower (WP_Post array) or an WP_Error
 	 */
 	public static function add_follower( $user_id, $actor ) {
 		$meta = get_remote_metadata_by_actor( $actor );
@@ -169,29 +169,30 @@ class Followers {
 			return $meta;
 		}
 
+		if ( empty( $meta ) || ! is_array( $meta ) || is_wp_error( $meta ) ) {
+			return new WP_Error( 'activitypub_invalid_follower', __( 'Invalid Follower', 'activitypub' ) );
+		}
+
 		$error = null;
 
 		$follower = new Follower();
+		$follower->from_array( $meta );
 
-		if ( empty( $meta ) || ! is_array( $meta ) || is_wp_error( $meta ) ) {
+		$id = $follower->upsert();
-			$follower->set_id( $actor );
+
-			$follower->set_url( $actor );
+		if ( is_wp_error( $id ) ) {
-			$error = $meta;
+			return $id;
-		} else {
-			$follower->from_array( $meta );
 		}
 
-		$follower->upsert();
+		$meta = get_post_meta( $id, 'activitypub_user_id' );
-
-		$meta = get_post_meta( $follower->get__id(), 'activitypub_user_id' );
 
 		if ( $error ) {
-			self::add_error( $follower->get__id(), $error );
+			self::add_error( $id, $error );
 		}
 
 		// phpcs:ignore WordPress.PHP.StrictInArray.MissingTrueStrict
 		if ( is_array( $meta ) && ! in_array( $user_id, $meta ) ) {
-			add_post_meta( $follower->get__id(), 'activitypub_user_id', $user_id );
+			add_post_meta( $id, 'activitypub_user_id', $user_id );
 			wp_cache_delete( sprintf( self::CACHE_KEY_INBOXES, $user_id ), 'activitypub' );
 		}
 

includes/functions.php

@@ -54,7 +54,7 @@ function get_remote_metadata_by_actor( $actor, $cached = true ) {
 	}
 
 	if ( ! $actor ) {
-		return null;
+		return new WP_Error( 'activitypub_no_valid_actor_identifier', \__( 'The "actor" identifier is not valid', 'activitypub' ), $actor );
 	}
 
 	if ( is_wp_error( $actor ) ) {
@@ -73,7 +73,7 @@ function get_remote_metadata_by_actor( $actor, $cached = true ) {
 	}
 
 	if ( ! \wp_http_validate_url( $actor ) ) {
-		$metadata = new \WP_Error( 'activitypub_no_valid_actor_url', \__( 'The "actor" is no valid URL', 'activitypub' ), $actor );
+		$metadata = new WP_Error( 'activitypub_no_valid_actor_url', \__( 'The "actor" is no valid URL', 'activitypub' ), $actor );
 		\set_transient( $transient_key, $metadata, HOUR_IN_SECONDS ); // Cache the error for a shorter period.
 		return $metadata;
 	}
@@ -95,7 +95,7 @@ function get_remote_metadata_by_actor( $actor, $cached = true ) {
 	\set_transient( $transient_key, $metadata, WEEK_IN_SECONDS );
 
 	if ( ! $metadata ) {
-		$metadata = new \WP_Error( 'activitypub_invalid_json', \__( 'No valid JSON data', 'activitypub' ), $actor );
+		$metadata = new WP_Error( 'activitypub_invalid_json', \__( 'No valid JSON data', 'activitypub' ), $actor );
 		\set_transient( $transient_key, $metadata, HOUR_IN_SECONDS ); // Cache the error for a shorter period.
 		return $metadata;
 	}

includes/model/class-follower.php

@@ -1,6 +1,7 @@
 <?php
 namespace Activitypub\Model;
 
+use WP_Error;
 use WP_Query;
 use Activitypub\Activity\Actor;
 use Activitypub\Collection\Followers;
@@ -110,12 +111,40 @@ class Follower extends Actor {
 		$this->save();
 	}
 
+	/**
+	 * Validate the current Follower-Object.
+	 *
+	 * @return boolean True if the verification was successful.
+	 */
+	public function is_valid() {
+		// the minimum required attributes
+		$required_attributes = array(
+			'id',
+			'preferredUsername',
+			'inbox',
+			'publicKey',
+			'publicKeyPem',
+		);
+
+		foreach ( $required_attributes as $attribute ) {
+			if ( ! $this->get( $attribute ) ) {
+				return false;
+			}
+		}
+
+		return true;
+	}
+
 	/**
 	 * Save the current Follower-Object.
 	 *
-	 * @return void
+	 * @return int|WP_Error The Post-ID or an WP_Error.
 	 */
 	public function save() {
+		if ( ! $this->is_valid() ) {
+			return new WP_Error( 'activitypub_invalid_follower', __( 'Invalid Follower', 'activitypub' ) );
+		}
+
 		if ( ! $this->get__id() ) {
 			global $wpdb;
 
@@ -147,15 +176,17 @@ class Follower extends Actor {
 
 		$post_id = wp_insert_post( $args );
 		$this->_id = $post_id;
+
+		return $post_id;
 	}
 
 	/**
 	 * Upsert the current Follower-Object.
 	 *
-	 * @return void
+	 * @return int|WP_Error The Post-ID or an WP_Error.
 	 */
 	public function upsert() {
-		$this->save();
+		return $this->save();
 	}
 
 	/**

includes/rest/class-inbox.php

@@ -131,7 +131,7 @@ class Inbox {
 			return $user;
 		}
 
-		$data = $request->get_params();
+		$data = $request->get_json_params();
 		$type = $request->get_param( 'type' );
 		$type = \strtolower( $type );
 
@@ -149,7 +149,7 @@ class Inbox {
 	 * @return WP_REST_Response
 	 */
 	public static function shared_inbox_post( $request ) {
-		$data = $request->get_params();
+		$data = $request->get_json_params();
 		$type = $request->get_param( 'type' );
 		$users = self::extract_recipients( $data );
 
@@ -331,61 +331,6 @@ class Inbox {
 		return $params;
 	}
 
-	/**
-	 * Handles "Reaction" requests
-	 *
-	 * @param  array $object  The activity-object
-	 * @param  int   $user_id The id of the local blog-user
-	 */
-	public static function handle_reaction( $object, $user_id ) {
-		$meta = get_remote_metadata_by_actor( $object['actor'] );
-
-		$comment_post_id = \url_to_postid( $object['object'] );
-
-		// save only replys and reactions
-		if ( ! $comment_post_id ) {
-			return false;
-		}
-
-		$commentdata = array(
-			'comment_post_ID' => $comment_post_id,
-			'comment_author' => \esc_attr( $meta['name'] ),
-			'comment_author_email' => '',
-			'comment_author_url' => \esc_url_raw( $object['actor'] ),
-			'comment_content' => \esc_url_raw( $object['actor'] ),
-			'comment_type' => \esc_attr( \strtolower( $object['type'] ) ),
-			'comment_parent' => 0,
-			'comment_meta' => array(
-				'source_url' => \esc_url_raw( $object['id'] ),
-				'avatar_url' => \esc_url_raw( $meta['icon']['url'] ),
-				'protocol' => 'activitypub',
-			),
-		);
-
-		// disable flood control
-		\remove_action( 'check_comment_flood', 'check_comment_flood_db', 10 );
-
-		// do not require email for AP entries
-		\add_filter( 'pre_option_require_name_email', '__return_false' );
-
-		// No nonce possible for this submission route
-		\add_filter(
-			'akismet_comment_nonce',
-			function() {
-				return 'inactive';
-			}
-		);
-
-		$state = \wp_new_comment( $commentdata, true );
-
-		\remove_filter( 'pre_option_require_name_email', '__return_false' );
-
-		// re-add flood control
-		\add_action( 'check_comment_flood', 'check_comment_flood_db', 10, 4 );
-
-		do_action( 'activitypub_handled_reaction', $object, $user_id, $state, $commentdata );
-	}
-
 	/**
 	 * Handles "Create" requests
 	 *

tests/test-class-db-activitypub-followers.php

@@ -43,6 +43,11 @@ class Test_Db_Activitypub_Followers extends WP_UnitTestCase {
 			'name'  => 'úser2',
 			'preferredUsername'  => 'user2',
 		),
+		'error@example.com' => array(
+			'url' => 'https://error.example.com',
+			'name'  => 'error',
+			'preferredUsername'  => 'error',
+		),
 	);
 
 	public function set_up() {
@@ -97,6 +102,27 @@ class Test_Db_Activitypub_Followers extends WP_UnitTestCase {
 		$this->assertContains( $follower2, $db_followers2 );
 	}
 
+	public function test_add_follower_error() {
+		$pre_http_request = new MockAction();
+		add_filter( 'pre_http_request', array( $pre_http_request, 'filter' ), 10, 3 );
+
+		$follower = 'error@example.com';
+
+		$result = \Activitypub\Collection\Followers::add_follower( 1, $follower );
+
+		$this->assertTrue( is_wp_error( $result ) );
+
+		$follower2 = 'https://error.example.com';
+
+		$result = \Activitypub\Collection\Followers::add_follower( 1, $follower2 );
+
+		$this->assertTrue( is_wp_error( $result ) );
+
+		$db_followers = \Activitypub\Collection\Followers::get_followers( 1 );
+
+		$this->assertEmpty( $db_followers );
+	}
+
 	public function test_get_follower() {
 		$followers = array( 'https://example.com/author/jon' );
 		$followers2 = array( 'https://user2.example.com' );
@@ -268,6 +294,30 @@ class Test_Db_Activitypub_Followers extends WP_UnitTestCase {
 		$this->assertCount( 1, $meta );
 	}
 
+	public function test_migration() {
+		$pre_http_request = new MockAction();
+		add_filter( 'pre_http_request', array( $pre_http_request, 'filter' ), 10, 3 );
+
+		$followers = array(
+			'https://example.com/author/jon',
+			'https://example.og/errors',
+			'https://example.org/author/doe',
+			'http://sally.example.org',
+			'https://error.example.com',
+			'https://example.net/error',
+		);
+
+		$user_id = 1;
+
+		add_user_meta( $user_id, 'activitypub_followers', $followers, true );
+
+		\Activitypub\Migration::maybe_migrate();
+
+		$db_followers = \Activitypub\Collection\Followers::get_followers( 1 );
+
+		$this->assertCount( 3, $db_followers );
+	}
+
 	/**
 	 * @dataProvider extract_name_from_uri_content_provider
 	 */