From ff955f17ded474340d1dffa7b29244dc999500c0 Mon Sep 17 00:00:00 2001 From: Darin Kotter Date: Mon, 11 Nov 2024 09:56:28 -0700 Subject: [PATCH 1/6] Ensure we verify the media_id key is set before we use it in our REST update requests, otherwise this causes an error. Also ensure this value is a number so we don't allow arbitrary strings here --- includes/class-simple-local-avatars.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/includes/class-simple-local-avatars.php b/includes/class-simple-local-avatars.php index 97fa80b..e8b0801 100644 --- a/includes/class-simple-local-avatars.php +++ b/includes/class-simple-local-avatars.php @@ -1305,6 +1305,14 @@ public function get_avatar_rest( $user ) { * @param object $user The user making the request. */ public function set_avatar_rest( $input, $user ) { + // Ensure media_id is set and is a number. + if ( + empty( $input['media_id'] ) || + ! is_numeric( $input['media_id'] ) + ) { + return; + } + $this->assign_new_user_avatar( $input['media_id'], $user->ID ); } From c7099c2949a485a5e8d1bd1d0c0ce5fc71b2b582 Mon Sep 17 00:00:00 2001 From: Darin Kotter Date: Mon, 11 Nov 2024 09:56:58 -0700 Subject: [PATCH 2/6] Remove the blog_id key from our avatar data before deleting, as there's no reason to try deleting a file based on that --- includes/class-simple-local-avatars.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/includes/class-simple-local-avatars.php b/includes/class-simple-local-avatars.php index e8b0801..f770d6b 100644 --- a/includes/class-simple-local-avatars.php +++ b/includes/class-simple-local-avatars.php @@ -1206,6 +1206,11 @@ public function avatar_delete( $user_id ) { unset( $old_avatars['media_id'], $old_avatars['full'] ); } + // Remove the blog_id key as we don't need to try deleting a file based on. + if ( array_key_exists( 'blog_id', $old_avatars ) ) { + unset( $old_avatars['blog_id'] ); + } + if ( ! empty( $old_avatars ) ) { $upload_path = wp_upload_dir(); From 6480f9e9bd8b9abeb594948a6569acbcc0211da6 Mon Sep 17 00:00:00 2001 From: Darin Kotter Date: Mon, 11 Nov 2024 09:57:10 -0700 Subject: [PATCH 3/6] Add safety check before deleting an avatar to ensure we only delete something if it's in the uploads directory --- includes/class-simple-local-avatars.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/includes/class-simple-local-avatars.php b/includes/class-simple-local-avatars.php index f770d6b..7e7c6f0 100644 --- a/includes/class-simple-local-avatars.php +++ b/includes/class-simple-local-avatars.php @@ -1215,6 +1215,11 @@ public function avatar_delete( $user_id ) { $upload_path = wp_upload_dir(); foreach ( $old_avatars as $old_avatar ) { + // Ensure the avatar is in the uploads directory before we delete it. + if ( strpos( $old_avatar, $upload_path['baseurl'] ) !== 0 ) { + continue; + } + // derive the path for the file based on the upload directory $old_avatar_path = str_replace( $upload_path['baseurl'], $upload_path['basedir'], $old_avatar ); if ( file_exists( $old_avatar_path ) ) { From 897e0d11260dbbcd0053fec700a153e8fb1b89d9 Mon Sep 17 00:00:00 2001 From: Darin Kotter Date: Mon, 11 Nov 2024 09:57:30 -0700 Subject: [PATCH 4/6] One extra check to ensure the media_id actually matches a valid attachment --- includes/class-simple-local-avatars.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/includes/class-simple-local-avatars.php b/includes/class-simple-local-avatars.php index 7e7c6f0..951638c 100644 --- a/includes/class-simple-local-avatars.php +++ b/includes/class-simple-local-avatars.php @@ -1323,7 +1323,12 @@ public function set_avatar_rest( $input, $user ) { return; } - $this->assign_new_user_avatar( $input['media_id'], $user->ID ); + // Ensure this media_id is a valid attachment. + if ( ! wp_get_attachment_url( (int) $input['media_id'] ) ) { + return; + } + + $this->assign_new_user_avatar( (int) $input['media_id'], $user->ID ); } /** From 900997d5a48cfe42281a885fba7c043dc92d2def Mon Sep 17 00:00:00 2001 From: Darin Kotter Date: Mon, 11 Nov 2024 10:05:15 -0700 Subject: [PATCH 5/6] Update comment --- includes/class-simple-local-avatars.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/class-simple-local-avatars.php b/includes/class-simple-local-avatars.php index 951638c..cfb190f 100644 --- a/includes/class-simple-local-avatars.php +++ b/includes/class-simple-local-avatars.php @@ -1206,7 +1206,7 @@ public function avatar_delete( $user_id ) { unset( $old_avatars['media_id'], $old_avatars['full'] ); } - // Remove the blog_id key as we don't need to try deleting a file based on. + // Remove the blog_id key as we don't need to try deleting a file based on that. if ( array_key_exists( 'blog_id', $old_avatars ) ) { unset( $old_avatars['blog_id'] ); } From 515c5160d90074e1e1cabefb8c36a3968538903c Mon Sep 17 00:00:00 2001 From: Darin Kotter Date: Tue, 12 Nov 2024 08:49:53 -0700 Subject: [PATCH 6/6] If our media_id checks don't pass, show an error message --- includes/class-simple-local-avatars.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/includes/class-simple-local-avatars.php b/includes/class-simple-local-avatars.php index cfb190f..c864ec9 100644 --- a/includes/class-simple-local-avatars.php +++ b/includes/class-simple-local-avatars.php @@ -1313,6 +1313,7 @@ public function get_avatar_rest( $user ) { * * @param array $input Input submitted via REST request. * @param object $user The user making the request. + * @return null|\WP_Error */ public function set_avatar_rest( $input, $user ) { // Ensure media_id is set and is a number. @@ -1320,12 +1321,12 @@ public function set_avatar_rest( $input, $user ) { empty( $input['media_id'] ) || ! is_numeric( $input['media_id'] ) ) { - return; + return new \WP_Error( 'invalid_media_id', esc_html__( 'Request did not contain a valid media_id field.', 'simple-local-avatars' ) ); } // Ensure this media_id is a valid attachment. if ( ! wp_get_attachment_url( (int) $input['media_id'] ) ) { - return; + return new \WP_Error( 'invalid_media_id', esc_html__( 'Media ID did not match a valid attachment.', 'simple-local-avatars' ) ); } $this->assign_new_user_avatar( (int) $input['media_id'], $user->ID );