From 1d779c6193ff445cd5db230913273e5a7ed8474d Mon Sep 17 00:00:00 2001
From: Hypolite Petovan <hypolite@mrpetovan.com>
Date: Tue, 22 Feb 2022 10:44:30 -0500
Subject: [PATCH] Ensure the private message recipient is valid in Mail::send

- Arbitrary input could be used to circumvent most restrictions regarding recipients (except contact relationship)
---
 src/Core/ACL.php   | 41 +++++++++++++++++++++--------------------
 src/Model/Mail.php | 31 +++++++++++++++----------------
 2 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/src/Core/ACL.php b/src/Core/ACL.php
index a4acf58bad..40612fadfc 100644
--- a/src/Core/ACL.php
+++ b/src/Core/ACL.php
@@ -51,7 +51,7 @@ class ACL
 	 * @return string
 	 * @throws \Exception
 	 */
-	public static function getMessageContactSelectHTML(int $selected = null)
+	public static function getMessageContactSelectHTML(int $selected = null): string
 	{
 		$o = '';
 
@@ -62,25 +62,7 @@ class ACL
 		$page->registerStylesheet(Theme::getPathForFile('js/friendica-tagsinput/friendica-tagsinput.css'));
 		$page->registerStylesheet(Theme::getPathForFile('js/friendica-tagsinput/friendica-tagsinput-typeahead.css'));
 
-		$condition = [
-			'uid' => local_user(),
-			'self' => false,
-			'blocked' => false,
-			'pending' => false,
-			'archive' => false,
-			'deleted' => false,
-			'rel' => [Contact::FOLLOWER, Contact::SHARING, Contact::FRIEND],
-			'network' => Protocol::SUPPORT_PRIVATE,
-		];
-
-		$contacts = Contact::selectToArray(
-			['id', 'name', 'addr', 'micro'],
-			DBA::mergeConditions($condition, ["`notify` != ''"])
-		);
-
-		$arr = ['contact' => $contacts, 'entry' => $o];
-
-		Hook::callAll(DI::args()->getModuleName() . '_pre_recipient', $arr);
+		$contacts = self::getValidMessageRecipientsForUser(local_user());
 
 		$tpl = Renderer::getMarkupTemplate('acl/message_recipient.tpl');
 		$o = Renderer::replaceMacros($tpl, [
@@ -93,6 +75,25 @@ class ACL
 		return $o;
 	}
 
+	public static function getValidMessageRecipientsForUser(int $uid): array
+	{
+		$condition = [
+			'uid'     => $uid,
+			'self'    => false,
+			'blocked' => false,
+			'pending' => false,
+			'archive' => false,
+			'deleted' => false,
+			'rel'     => [Contact::FOLLOWER, Contact::SHARING, Contact::FRIEND],
+			'network' => Protocol::SUPPORT_PRIVATE,
+		];
+
+		return Contact::selectToArray(
+			['id', 'name', 'addr', 'micro', 'url', 'nick'],
+			DBA::mergeConditions($condition, ["`notify` != ''"])
+		);
+	}
+
 	/**
 	 * Returns a minimal ACL block for self-only permissions
 	 *
diff --git a/src/Model/Mail.php b/src/Model/Mail.php
index 79a0b5f72b..e82a01fbce 100644
--- a/src/Model/Mail.php
+++ b/src/Model/Mail.php
@@ -21,6 +21,7 @@
 
 namespace Friendica\Model;
 
+use Friendica\Core\ACL;
 use Friendica\Core\Logger;
 use Friendica\Core\System;
 use Friendica\Core\Worker;
@@ -39,10 +40,12 @@ class Mail
 	 * Insert private message
 	 *
 	 * @param array $msg
-	 * @param bool  $notifiction
+	 * @param bool  $notification
 	 * @return int|boolean Message ID or false on error
+	 * @throws \Friendica\Network\HTTPException\InternalServerErrorException
+	 * @throws \ImagickException
 	 */
-	public static function insert($msg, $notifiction = true)
+	public static function insert($msg, $notification = true)
 	{
 		if (!isset($msg['reply'])) {
 			$msg['reply'] = DBA::exists('mail', ['parent-uri' => $msg['parent-uri']]);
@@ -92,7 +95,7 @@ class Mail
 			DBA::update('conv', ['updated' => DateTimeFormat::utcNow()], ['id' => $msg['convid']]);
 		}
 
-		if ($notifiction) {
+		if ($notification) {
 			$user = User::getById($msg['uid']);
 			// send notifications.
 			$notif_params = [
@@ -139,11 +142,15 @@ class Mail
 			return -2;
 		}
 
-		$contact = DBA::selectFirst('contact', [], ['id' => $recipient, 'uid' => local_user()]);
-		if (!DBA::isResult($contact)) {
+		$contacts = ACL::getValidMessageRecipientsForUser(local_user());
+
+		$contactIndex = array_search($recipient, array_column($contacts, 'id'));
+		if ($contactIndex === false) {
 			return -2;
 		}
 
+		$contact = $contacts[$contactIndex];
+
 		Photo::setPermissionFromBody($body, local_user(), $me['id'],  '<' . $contact['id'] . '>', '', '', '');
 
 		$guid = System::createUUID();
@@ -167,20 +174,12 @@ class Mail
 		$convuri = '';
 		if (!$convid) {
 			// create a new conversation
-			$recip_host = substr($contact['url'], strpos($contact['url'], '://') + 3);
-			$recip_host = substr($recip_host, 0, strpos($recip_host, '/'));
-
-			$recip_handle = (($contact['addr']) ? $contact['addr'] : $contact['nick'] . '@' . $recip_host);
-			$sender_handle = $a->getLoggedInUserNickname() . '@' . substr(DI::baseUrl(), strpos(DI::baseUrl(), '://') + 3);
-
 			$conv_guid = System::createUUID();
-			$convuri = $recip_handle . ':' . $conv_guid;
+			$convuri = $contact['addr'] . ':' . $conv_guid;
 
-			$handles = $recip_handle . ';' . $sender_handle;
-
-			$fields = ['uid' => local_user(), 'guid' => $conv_guid, 'creator' => $sender_handle,
+			$fields = ['uid' => local_user(), 'guid' => $conv_guid, 'creator' => $me['addr'],
 				'created' => DateTimeFormat::utcNow(), 'updated' => DateTimeFormat::utcNow(),
-				'subject' => $subject, 'recips' => $handles];
+				'subject' => $subject, 'recips' => $contact['addr'] . ';' . $me['addr']];
 			if (DBA::insert('conv', $fields)) {
 				$convid = DBA::lastInsertId();
 			}