Skip to content

Commit 8dcbe0c

Browse files
pfefferlepfefferle
authored
fix Secops issues (#411)
1 parent 2ad9bf9 commit 8dcbe0c

File tree

8 files changed

+106
-79
lines changed

8 files changed

+106
-79
lines changed

‎includes/class-mention.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public static function the_content( $the_content ) {
7474
public static function replace_with_links( $result ) {
7575
$metadata = get_remote_metadata_by_actor( $result[0] );
7676

77-
if ( ! is_wp_error( $metadata ) && ! empty( $metadata['url'] ) ) {
77+
if ( ! empty( $metadata ) && ! is_wp_error( $metadata ) && ! empty( $metadata['url'] ) ) {
7878
$username = ltrim( $result[0], '@' );
7979
if ( ! empty( $metadata['name'] ) ) {
8080
$username = $metadata['name'];

‎includes/class-shortcodes.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ public static function excerpt( $atts, $content, $tag ) {
114114

115115
/** This filter is documented in wp-includes/post-template.php */
116116
$excerpt = \apply_filters( 'the_content', $excerpt );
117-
$excerpt = \str_replace( ']]>', ']]>', $excerpt );
117+
$excerpt = \str_replace( ']]>', ']]>', $excerpt );
118118
}
119119
}
120120

‎includes/class-signature.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ public static function verify_http_signature( $request ) {
234234
*
235235
* @param string $key_id The URL to the public key.
236236
*
237-
* @return string The public key.
237+
* @return WP_Error|string The public key.
238238
*/
239239
public static function get_remote_key( $key_id ) { // phpcs:ignore
240240
$actor = get_remote_metadata_by_actor( strip_fragment_from_url( $key_id ) ); // phpcs:ignore
@@ -244,7 +244,7 @@ public static function get_remote_key( $key_id ) { // phpcs:ignore
244244
if ( isset( $actor['publicKey']['publicKeyPem'] ) ) {
245245
return \rtrim( $actor['publicKey']['publicKeyPem'] ); // phpcs:ignore
246246
}
247-
return null;
247+
return new WP_Error( 'activitypub_no_remote_key_found', 'No Public-Key found' );
248248
}
249249

250250
/**

‎includes/collection/class-followers.php‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ public static function handle_undo_request( $object, $user_id ) {
160160
* @param int $user_id The ID of the WordPress User
161161
* @param string $actor The Actor URL
162162
*
163-
* @return array|WP_Error The Follower (WP_Term array) or an WP_Error
163+
* @return array|WP_Error The Follower (WP_Post array) or an WP_Error
164164
*/
165165
public static function add_follower( $user_id, $actor ) {
166166
$meta = get_remote_metadata_by_actor( $actor );
@@ -169,29 +169,30 @@ public static function add_follower( $user_id, $actor ) {
169169
return $meta;
170170
}
171171

172+
if ( empty( $meta ) || ! is_array( $meta ) || is_wp_error( $meta ) ) {
173+
return new WP_Error( 'activitypub_invalid_follower', __( 'Invalid Follower', 'activitypub' ) );
174+
}
175+
172176
$error = null;
173177

174178
$follower = new Follower();
179+
$follower->from_array( $meta );
175180

176-
if ( empty( $meta ) || ! is_array( $meta ) || is_wp_error( $meta ) ) {
177-
$follower->set_id( $actor );
178-
$follower->set_url( $actor );
179-
$error = $meta;
180-
} else {
181-
$follower->from_array( $meta );
182-
}
181+
$id = $follower->upsert();
183182

184-
$follower->upsert();
183+
if ( is_wp_error( $id ) ) {
184+
return $id;
185+
}
185186

186-
$meta = get_post_meta( $follower->get__id(), 'activitypub_user_id' );
187+
$meta = get_post_meta( $id, 'activitypub_user_id' );
187188

188189
if ( $error ) {
189-
self::add_error( $follower->get__id(), $error );
190+
self::add_error( $id, $error );
190191
}
191192

192193
// phpcs:ignore WordPress.PHP.StrictInArray.MissingTrueStrict
193194
if ( is_array( $meta ) && ! in_array( $user_id, $meta ) ) {
194-
add_post_meta( $follower->get__id(), 'activitypub_user_id', $user_id );
195+
add_post_meta( $id, 'activitypub_user_id', $user_id );
195196
wp_cache_delete( sprintf( self::CACHE_KEY_INBOXES, $user_id ), 'activitypub' );
196197
}
197198

‎includes/functions.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ function get_remote_metadata_by_actor( $actor, $cached = true ) {
5454
}
5555

5656
if ( ! $actor ) {
57-
return null;
57+
return new WP_Error( 'activitypub_no_valid_actor_identifier', \__( 'The "actor" identifier is not valid', 'activitypub' ), $actor );
5858
}
5959

6060
if ( is_wp_error( $actor ) ) {
@@ -73,7 +73,7 @@ function get_remote_metadata_by_actor( $actor, $cached = true ) {
7373
}
7474

7575
if ( ! \wp_http_validate_url( $actor ) ) {
76-
$metadata = new \WP_Error( 'activitypub_no_valid_actor_url', \__( 'The "actor" is no valid URL', 'activitypub' ), $actor );
76+
$metadata = new WP_Error( 'activitypub_no_valid_actor_url', \__( 'The "actor" is no valid URL', 'activitypub' ), $actor );
7777
\set_transient( $transient_key, $metadata, HOUR_IN_SECONDS ); // Cache the error for a shorter period.
7878
return $metadata;
7979
}
@@ -95,7 +95,7 @@ function get_remote_metadata_by_actor( $actor, $cached = true ) {
9595
\set_transient( $transient_key, $metadata, WEEK_IN_SECONDS );
9696

9797
if ( ! $metadata ) {
98-
$metadata = new \WP_Error( 'activitypub_invalid_json', \__( 'No valid JSON data', 'activitypub' ), $actor );
98+
$metadata = new WP_Error( 'activitypub_invalid_json', \__( 'No valid JSON data', 'activitypub' ), $actor );
9999
\set_transient( $transient_key, $metadata, HOUR_IN_SECONDS ); // Cache the error for a shorter period.
100100
return $metadata;
101101
}

‎includes/model/class-follower.php‎

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
<?php
22
namespace Activitypub\Model;
33

4+
use WP_Error;
45
use WP_Query;
56
use Activitypub\Activity\Actor;
67
use Activitypub\Collection\Followers;
@@ -110,12 +111,40 @@ public function update() {
110111
$this->save();
111112
}
112113

114+
/**
115+
* Validate the current Follower-Object.
116+
*
117+
* @return boolean True if the verification was successful.
118+
*/
119+
public function is_valid() {
120+
// the minimum required attributes
121+
$required_attributes = array(
122+
'id',
123+
'preferredUsername',
124+
'inbox',
125+
'publicKey',
126+
'publicKeyPem',
127+
);
128+
129+
foreach ( $required_attributes as $attribute ) {
130+
if ( ! $this->get( $attribute ) ) {
131+
return false;
132+
}
133+
}
134+
135+
return true;
136+
}
137+
113138
/**
114139
* Save the current Follower-Object.
115140
*
116-
* @return void
141+
* @return int|WP_Error The Post-ID or an WP_Error.
117142
*/
118143
public function save() {
144+
if ( ! $this->is_valid() ) {
145+
return new WP_Error( 'activitypub_invalid_follower', __( 'Invalid Follower', 'activitypub' ) );
146+
}
147+
119148
if ( ! $this->get__id() ) {
120149
global $wpdb;
121150

@@ -147,15 +176,17 @@ public function save() {
147176

148177
$post_id = wp_insert_post( $args );
149178
$this->_id = $post_id;
179+
180+
return $post_id;
150181
}
151182

152183
/**
153184
* Upsert the current Follower-Object.
154185
*
155-
* @return void
186+
* @return int|WP_Error The Post-ID or an WP_Error.
156187
*/
157188
public function upsert() {
158-
$this->save();
189+
return $this->save();
159190
}
160191

161192
/**

‎includes/rest/class-inbox.php‎

Lines changed: 2 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ public static function user_inbox_post( $request ) {
131131
return $user;
132132
}
133133

134-
$data = $request->get_params();
134+
$data = $request->get_json_params();
135135
$type = $request->get_param( 'type' );
136136
$type = \strtolower( $type );
137137

@@ -149,7 +149,7 @@ public static function user_inbox_post( $request ) {
149149
* @return WP_REST_Response
150150
*/
151151
public static function shared_inbox_post( $request ) {
152-
$data = $request->get_params();
152+
$data = $request->get_json_params();
153153
$type = $request->get_param( 'type' );
154154
$users = self::extract_recipients( $data );
155155

@@ -331,61 +331,6 @@ public static function shared_inbox_post_parameters() {
331331
return $params;
332332
}
333333

334-
/**
335-
* Handles "Reaction" requests
336-
*
337-
* @param array $object The activity-object
338-
* @param int $user_id The id of the local blog-user
339-
*/
340-
public static function handle_reaction( $object, $user_id ) {
341-
$meta = get_remote_metadata_by_actor( $object['actor'] );
342-
343-
$comment_post_id = \url_to_postid( $object['object'] );
344-
345-
// save only replys and reactions
346-
if ( ! $comment_post_id ) {
347-
return false;
348-
}
349-
350-
$commentdata = array(
351-
'comment_post_ID' => $comment_post_id,
352-
'comment_author' => \esc_attr( $meta['name'] ),
353-
'comment_author_email' => '',
354-
'comment_author_url' => \esc_url_raw( $object['actor'] ),
355-
'comment_content' => \esc_url_raw( $object['actor'] ),
356-
'comment_type' => \esc_attr( \strtolower( $object['type'] ) ),
357-
'comment_parent' => 0,
358-
'comment_meta' => array(
359-
'source_url' => \esc_url_raw( $object['id'] ),
360-
'avatar_url' => \esc_url_raw( $meta['icon']['url'] ),
361-
'protocol' => 'activitypub',
362-
),
363-
);
364-
365-
// disable flood control
366-
\remove_action( 'check_comment_flood', 'check_comment_flood_db', 10 );
367-
368-
// do not require email for AP entries
369-
\add_filter( 'pre_option_require_name_email', '__return_false' );
370-
371-
// No nonce possible for this submission route
372-
\add_filter(
373-
'akismet_comment_nonce',
374-
function() {
375-
return 'inactive';
376-
}
377-
);
378-
379-
$state = \wp_new_comment( $commentdata, true );
380-
381-
\remove_filter( 'pre_option_require_name_email', '__return_false' );
382-
383-
// re-add flood control
384-
\add_action( 'check_comment_flood', 'check_comment_flood_db', 10, 4 );
385-
386-
do_action( 'activitypub_handled_reaction', $object, $user_id, $state, $commentdata );
387-
}
388-
389334
/**
390335
* Handles "Create" requests
391336
*

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,11 @@ class Test_Db_Activitypub_Followers extends WP_UnitTestCase {
4343
'name' => 'úser2',
4444
'preferredUsername' => 'user2',
4545
),
46+
'[email protected]' => array(
47+
'url' => 'https://error.example.com',
48+
'name' => 'error',
49+
'preferredUsername' => 'error',
50+
),
4651
);
4752

4853
public function set_up() {
@@ -97,6 +102,27 @@ public function test_add_follower() {
97102
$this->assertContains( $follower2, $db_followers2 );
98103
}
99104

105+
public function test_add_follower_error() {
106+
$pre_http_request = new MockAction();
107+
add_filter( 'pre_http_request', array( $pre_http_request, 'filter' ), 10, 3 );
108+
109+
$follower = '[email protected]';
110+
111+
$result = \Activitypub\Collection\Followers::add_follower( 1, $follower );
112+
113+
$this->assertTrue( is_wp_error( $result ) );
114+
115+
$follower2 = 'https://error.example.com';
116+
117+
$result = \Activitypub\Collection\Followers::add_follower( 1, $follower2 );
118+
119+
$this->assertTrue( is_wp_error( $result ) );
120+
121+
$db_followers = \Activitypub\Collection\Followers::get_followers( 1 );
122+
123+
$this->assertEmpty( $db_followers );
124+
}
125+
100126
public function test_get_follower() {
101127
$followers = array( 'https://example.com/author/jon' );
102128
$followers2 = array( 'https://user2.example.com' );
@@ -268,6 +294,30 @@ public function test_add_duplicate_follower() {
268294
$this->assertCount( 1, $meta );
269295
}
270296

297+
public function test_migration() {
298+
$pre_http_request = new MockAction();
299+
add_filter( 'pre_http_request', array( $pre_http_request, 'filter' ), 10, 3 );
300+
301+
$followers = array(
302+
'https://example.com/author/jon',
303+
'https://example.og/errors',
304+
'https://example.org/author/doe',
305+
'http://sally.example.org',
306+
'https://error.example.com',
307+
'https://example.net/error',
308+
);
309+
310+
$user_id = 1;
311+
312+
add_user_meta( $user_id, 'activitypub_followers', $followers, true );
313+
314+
\Activitypub\Migration::maybe_migrate();
315+
316+
$db_followers = \Activitypub\Collection\Followers::get_followers( 1 );
317+
318+
$this->assertCount( 3, $db_followers );
319+
}
320+
271321
/**
272322
* @dataProvider extract_name_from_uri_content_provider
273323
*/

0 commit comments

Comments
 (0)