← All changes | includes/forms/class-charitable-profile-form.php +71 -20 1.8.11.1 → 1.8.13 View file →

		@@ -711,42 +711,93 @@
711	711	* @access public
712	712	* @since 1.0.0
713	713	*/
714	714	public function save_avatar( $submitted, $fields, $form ) {
715		- if ( isset( $_FILES ) && isset( $_FILES['avatar'] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Missing
	715	+ $user_id = (int) $form->get_user()->ID;
	716	+ $files_avatar = isset( $_FILES['avatar'] ) ? $_FILES['avatar'] : null; // phpcs:ignore WordPress.Security.NonceVerification.Missing,WordPress.Security.ValidatedSanitizedInput.InputNotSanitized,WordPress.Security.ValidatedSanitizedInput.MissingUnslash
	717	+ $has_upload = is_array( $files_avatar )
	718	+ && ! empty( $files_avatar['name'] )
	719	+ && UPLOAD_ERR_NO_FILE !== (int) ( $files_avatar['error'] ?? UPLOAD_ERR_NO_FILE );
716	720
	721	+ if ( $has_upload ) {
717	722	$attachment_id = $form->upload_post_attachment( 'avatar', 0 );
718	723
719		- if ( ! is_wp_error( $attachment_id ) ) {
	724	+ if ( is_wp_error( $attachment_id ) ) {
	725	+ return $submitted;
	726	+ }
720	727
721		- $submitted['avatar'] = $attachment_id;
	728	+ $submitted['avatar'] = (int) $attachment_id;
722	729
723		- /* Delete the previously upload avatar. */
724		- $old_avatar = get_user_meta( $form->get_user()->ID, 'avatar', true );
	730	+ // Delete the previously stored avatar — only if it belongs to this user.
	731	+ // Without this ownership check, a poisoned 'avatar' user-meta value
	732	+ // (planted by a no-file submission, see below) would chain into
	733	+ // wp_delete_attachment() on an attacker-chosen attachment ID.
	734	+ $old_avatar = get_user_meta( $user_id, 'avatar', true );
	735	+ if ( ! empty( $old_avatar )
	736	+ && $this->user_owns_attachment( $user_id, (int) $old_avatar ) ) {
	737	+ wp_delete_attachment( (int) $old_avatar );
	738	+ }
725	739
726		- if ( ! empty( $old_avatar ) ) {
	740	+ update_user_meta( $user_id, 'avatar', (int) $attachment_id );
727	741
728		- wp_delete_attachment( $old_avatar );
	742	+ return $submitted;
	743	+ }
729	744
730		- }
	745	+ // No file in $_FILES. The avatar uploader (plupload) uploads the image
	746	+ // asynchronously and posts the resulting attachment ID as a hidden field
	747	+ // value, so the normal "change avatar" flow lands here with the new ID and
	748	+ // no $_FILES — we must not reject it.
	749	+ //
	750	+ // Block meta poisoning: only persist an attachment the submitting user
	751	+ // actually owns (a no-op re-save of their current avatar is also allowed).
	752	+ // An attacker-supplied ID owned by someone else is discarded so it cannot
	753	+ // be stored in user meta and later weaponised into wp_delete_attachment()
	754	+ // on a subsequent $_FILES submission.
	755	+ $current = (int) get_user_meta( $user_id, 'avatar', true );
	756	+ $posted = array_key_exists( 'avatar', $submitted ) ? $submitted['avatar'] : null;
731	757
732		- update_user_meta( $form->get_user()->ID, 'avatar', $attachment_id );
	758	+ // Absent, empty, or non-scalar value means the avatar was cleared. The
	759	+ // non-scalar guard blocks array poisoning (e.g. avatar[]=foo would
	760	+ // otherwise coerce via (int) to 1 and bypass the ownership check below).
	761	+ if ( null === $posted || '' === $posted || ! is_scalar( $posted ) ) {
	762	+ /* The picture has been removed. */
	763	+ $submitted['avatar'] = '';
	764	+ return $submitted;
	765	+ }
733	766
734		- } else {
735		- /**
736		- * Handle image upload error.
737		- *
738		- * @todo
739		- */
740		- }//end if
741		- } elseif ( ! array_key_exists( 'avatar', $submitted ) ) {
	767	+ $posted = (int) $posted;
742	768
743		- /* The picture has been removed. */
744		- $submitted['avatar'] = '';
	769	+ // Discard anything that is neither the current avatar nor an attachment
	770	+ // this user owns.
	771	+ if ( $posted !== $current && ! $this->user_owns_attachment( $user_id, $posted ) ) {
	772	+ $submitted['avatar'] = $current ?: '';
	773	+ return $submitted;
	774	+ }
745	775
746		- }//end if
	776	+ $submitted['avatar'] = $posted;
747	777
748	778	return $submitted;
	779	+ }
	780	+
	781	+ /**
	782	+ * Verify the given user owns the given attachment.
	783	+ *
	784	+ * @since 1.8.11.2
	785	+ *
	786	+ * @param int $user_id The user ID.
	787	+ * @param int $attachment_id The attachment ID.
	788	+ * @return bool
	789	+ */
	790	+ private function user_owns_attachment( $user_id, $attachment_id ) {
	791	+ if ( $user_id <= 0 || $attachment_id <= 0 ) {
	792	+ return false;
	793	+ }
	794	+
	795	+ $attachment = get_post( $attachment_id );
	796	+
	797	+ return $attachment instanceof WP_Post
	798	+ && 'attachment' === $attachment->post_type
	799	+ && (int) $attachment->post_author === (int) $user_id;
749	800	}
750	801	}
751	802
752	803	endif;