From ba4860b7879f0dc6ea7a2dd0d09e698a913a9129 Mon Sep 17 00:00:00 2001 From: Michael Date: Fri, 27 Jan 2023 05:55:45 +0000 Subject: [PATCH 1/2] Avoid local network communication / invalid url requests --- src/Content/Text/BBCode.php | 2 +- src/Model/APContact.php | 5 ++ src/Model/Contact.php | 20 +++++-- src/Model/Post/Media.php | 2 +- src/Model/Tag.php | 3 +- src/Module/Photo.php | 44 ++++++++++++-- src/Network/Probe.php | 16 +++-- src/Protocol/ActivityPub/Receiver.php | 4 ++ src/Protocol/Feed.php | 2 +- src/Protocol/OStatus.php | 85 +-------------------------- src/Worker/Notifier.php | 12 +++- src/Worker/OnePoll.php | 6 ++ 12 files changed, 96 insertions(+), 105 deletions(-) diff --git a/src/Content/Text/BBCode.php b/src/Content/Text/BBCode.php index c6fd7f8c5e..83056b2692 100644 --- a/src/Content/Text/BBCode.php +++ b/src/Content/Text/BBCode.php @@ -305,7 +305,7 @@ class BBCode // if nothing is found, it maybe having an image. if (!isset($post['type'])) { if (preg_match_all("#\[url=([^\]]+?)\]\s*\[img\]([^\[]+?)\[/img\]\s*\[/url\]#ism", $post['text'], $pictures, PREG_SET_ORDER)) { - if ((count($pictures) == 1) && !$has_title) { + if ((count($pictures) == 1) && !$has_title && !Photo::isLocal($pictures[0][2])) { if (!empty($item['object-type']) && ($item['object-type'] == Activity\ObjectType::IMAGE)) { // Replace the preview picture with the real picture $url = str_replace('-1.', '-0.', $pictures[0][2]); diff --git a/src/Model/APContact.php b/src/Model/APContact.php index 09a6cb9e78..072dea9d3e 100644 --- a/src/Model/APContact.php +++ b/src/Model/APContact.php @@ -376,6 +376,11 @@ class APContact // Unhandled from Kroeg // kroeg:blocks, updated + if (!empty($apcontact['photo']) && !Network::isValidHttpUrl($apcontact['photo'])) { + Logger::info('Invalid URL for photo', ['url' => $apcontact['url'], 'photo' => $apcontact['photo']]); + $apcontact['photo'] = null; + } + // When the photo is too large, try to shorten it by removing parts if (strlen($apcontact['photo'] ?? '') > 255) { $parts = parse_url($apcontact['photo']); diff --git a/src/Model/Contact.php b/src/Model/Contact.php index 7b5a1e428a..1408e4a0cb 100644 --- a/src/Model/Contact.php +++ b/src/Model/Contact.php @@ -2210,14 +2210,22 @@ class Contact if (($uid == 0) && !$force && empty($contact['thumb']) && empty($contact['micro']) && !$create_cache) { if (($contact['avatar'] != $avatar) || empty($contact['blurhash'])) { $update_fields = ['avatar' => $avatar]; - $fetchResult = HTTPSignature::fetchRaw($avatar, 0, [HttpClientOptions::ACCEPT_CONTENT => [HttpClientAccept::IMAGE]]); + if (!Network::isLocalLink($avatar) && Network::isValidHttpUrl($avatar)) { + $fetchResult = HTTPSignature::fetchRaw($avatar, 0, [HttpClientOptions::ACCEPT_CONTENT => [HttpClientAccept::IMAGE]]); - $img_str = $fetchResult->getBody(); - if (!empty($img_str)) { - $image = new Image($img_str, Images::getMimeTypeByData($img_str)); - if ($image->isValid()) { - $update_fields['blurhash'] = $image->getBlurHash(); + $img_str = $fetchResult->getBody(); + if (!empty($img_str)) { + $image = new Image($img_str, Images::getMimeTypeByData($img_str)); + if ($image->isValid()) { + $update_fields['blurhash'] = $image->getBlurHash(); + } else { + return; + } } + } elseif (!empty($contact['blurhash'])) { + $update_fields['blurhash'] = null; + } else { + return; } self::update($update_fields, ['id' => $cid]); diff --git a/src/Model/Post/Media.php b/src/Model/Post/Media.php index ef03b5fea5..8216d73837 100644 --- a/src/Model/Post/Media.php +++ b/src/Model/Post/Media.php @@ -180,7 +180,7 @@ class Media } // Fetch the mimetype or size if missing. - if (empty($media['mimetype']) || empty($media['size'])) { + if (Network::isValidHttpUrl($media['url']) && (empty($media['mimetype']) || empty($media['size']))) { $timeout = DI::config()->get('system', 'xrd_timeout'); $curlResult = DI::httpClient()->head($media['url'], [HttpClientOptions::TIMEOUT => $timeout]); diff --git a/src/Model/Tag.php b/src/Model/Tag.php index 8509c30938..897a39e60e 100644 --- a/src/Model/Tag.php +++ b/src/Model/Tag.php @@ -31,6 +31,7 @@ use Friendica\Database\DBA; use Friendica\DI; use Friendica\Protocol\ActivityPub; use Friendica\Util\DateTimeFormat; +use Friendica\Util\Network; use Friendica\Util\Strings; /** @@ -193,7 +194,7 @@ class Tag } elseif (Contact::getIdForURL($url, 0, $fetch ? null : false)) { $target = self::ACCOUNT; Logger::debug('URL is an account', ['url' => $url]); - } elseif ($fetch && ($target != self::GENERAL_COLLECTION)) { + } elseif ($fetch && ($target != self::GENERAL_COLLECTION) && Network::isValidHttpUrl($url)) { $content = ActivityPub::fetchContent($url); if (!empty($content['type']) && ($content['type'] == 'OrderedCollection')) { $target = self::GENERAL_COLLECTION; diff --git a/src/Module/Photo.php b/src/Module/Photo.php index 687ba98c74..7f3429b028 100644 --- a/src/Module/Photo.php +++ b/src/Module/Photo.php @@ -349,6 +349,13 @@ class Photo extends BaseModule } elseif (!empty($contact['avatar'])) { $url = $contact['avatar']; } + + // If it is a local link, we save resources by just redirecting to it. + if (Network::isLocalLink($url)) { + System::externalRedirect($url); + System::exit(); + } + $mimetext = ''; if (!empty($url)) { $mime = ParseUrl::getContentType($url, HttpClientAccept::IMAGE); @@ -393,6 +400,10 @@ class Photo extends BaseModule } else { $url = Contact::getDefaultAvatar($contact ?: [], Proxy::SIZE_SMALL); } + if (Network::isLocalLink($url)) { + System::externalRedirect($url); + System::exit(); + } } return MPhoto::createPhotoForExternalResource($url, 0, $mimetext, $contact['blurhash'] ?? null, $customsize, $customsize); case 'header': @@ -401,6 +412,15 @@ class Photo extends BaseModule if (empty($contact)) { return false; } + + if (Network::isLocalLink($contact['url'])) { + $header_uid = User::getIdForURL($contact['url']); + if (empty($header_uid)) { + throw new HTTPException\NotFoundException(); + } + return self::getBannerForUser($header_uid); + } + If (($contact['uid'] != 0) && empty($contact['header'])) { $contact = Contact::getByURL($contact['url'], false, $fields); } @@ -408,14 +428,14 @@ class Photo extends BaseModule $url = $contact['header']; } else { $url = Contact::getDefaultHeader($contact); + if (Network::isLocalLink($url)) { + System::externalRedirect($url); + System::exit(); + } } return MPhoto::createPhotoForExternalResource($url); case 'banner': - $photo = MPhoto::selectFirst([], ['scale' => 3, 'uid' => $id, 'photo-type' => MPhoto::USER_BANNER]); - if (!empty($photo)) { - return $photo; - } - return MPhoto::createPhotoForExternalResource(DI::baseUrl() . '/images/friendica-banner.jpg'); + return self::getBannerForUser($id); case 'profile': case 'custom': $scale = 4; @@ -445,6 +465,11 @@ class Photo extends BaseModule $default = Contact::getDefaultAvatar($contact, Proxy::SIZE_THUMB); } + if (Network::isLocalLink($default)) { + System::externalRedirect($default); + System::exit(); + } + $parts = parse_url($default); if (!empty($parts['scheme']) || !empty($parts['host'])) { $photo = MPhoto::createPhotoForExternalResource($default); @@ -454,4 +479,13 @@ class Photo extends BaseModule } return $photo; } + + private static function getBannerForUser(int $uid): array + { + $photo = MPhoto::selectFirst([], ['scale' => 3, 'uid' => $uid, 'photo-type' => MPhoto::USER_BANNER]); + if (!empty($photo)) { + return $photo; + } + return MPhoto::createPhotoForImageData(file_get_contents(DI::basePath() . '/images/friendica-banner.jpg')); + } } diff --git a/src/Network/Probe.php b/src/Network/Probe.php index d35490d0d3..e1bedf5e53 100644 --- a/src/Network/Probe.php +++ b/src/Network/Probe.php @@ -120,6 +120,11 @@ class Probe $numeric_fields = ['gsid', 'hide', 'account-type', 'manually-approve']; + if (!empty($data['photo']) && !Network::isValidHttpUrl($data['photo'])) { + Logger::info('Invalid URL for photo', ['url' => $data['url'], 'photo' => $data['photo']]); + unset($data['photo']); + } + $newdata = []; foreach ($fields as $field) { if (isset($data[$field])) { @@ -755,7 +760,7 @@ class Probe $result = self::zot($webfinger, $result, $baseurl); } if ((!$result && ($network == '')) || ($network == Protocol::PUMPIO)) { - $result = self::pumpio($webfinger, $addr); + $result = self::pumpio($webfinger, $addr, $baseurl); } if (empty($result['network']) && empty($ap_profile['network']) || ($network == Protocol::FEED)) { $result = self::feed($uri); @@ -1635,7 +1640,7 @@ class Probe * * @return array Profile data */ - private static function pumpioProfileData(string $profile_link): array + private static function pumpioProfileData(string $profile_link, string $baseurl): array { $curlResult = DI::httpClient()->get($profile_link, HttpClientAccept::HTML); if (!$curlResult->isSuccess() || empty($curlResult->getBody())) { @@ -1681,6 +1686,9 @@ class Probe foreach ($avatar->attributes as $attribute) { if ($attribute->name == 'src') { $data['photo'] = trim($attribute->value); + if (!empty($data['photo']) && !parse_url($data['photo'], PHP_URL_SCHEME) && !parse_url($data['photo'], PHP_URL_HOST)) { + $data['photo'] = $baseurl . $data['photo']; + } } } } @@ -1696,7 +1704,7 @@ class Probe * * @return array pump.io data */ - private static function pumpio(array $webfinger, string $addr): array + private static function pumpio(array $webfinger, string $addr, string $baseurl): array { $data = []; // The array is reversed to take into account the order of preference for same-rel links @@ -1728,7 +1736,7 @@ class Probe return []; } - $profile_data = self::pumpioProfileData($data['url']); + $profile_data = self::pumpioProfileData($data['url'], $baseurl); if (!$profile_data) { return []; diff --git a/src/Protocol/ActivityPub/Receiver.php b/src/Protocol/ActivityPub/Receiver.php index 2fad302983..6bcf97057a 100644 --- a/src/Protocol/ActivityPub/Receiver.php +++ b/src/Protocol/ActivityPub/Receiver.php @@ -1065,6 +1065,10 @@ class Receiver } foreach ($receiver_list as $receiver) { + if ($receiver == 'Public') { + Logger::notice('Not compacted public collection found', ['activity' => $activity, 'callstack' => System::callstack(20)]); + $receiver = ActivityPub::PUBLIC_COLLECTION; + } if ($receiver == self::PUBLIC_COLLECTION) { $receiver = ActivityPub::PUBLIC_COLLECTION; } diff --git a/src/Protocol/Feed.php b/src/Protocol/Feed.php index be58b9aa8f..0abd49c462 100644 --- a/src/Protocol/Feed.php +++ b/src/Protocol/Feed.php @@ -1122,7 +1122,7 @@ class Feed XML::addElement($doc, $entry, 'id', $item['uri']); XML::addElement($doc, $entry, 'title', html_entity_decode($title, ENT_QUOTES, 'UTF-8')); - $body = OStatus::formatPicturePost($item['body'], $item['uri-id']); + $body = Post\Media::addAttachmentsToBody($item['uri-id'], DI::contentItem()->addSharedPost($item)); $body = BBCode::convertForUriId($item['uri-id'], $body, BBCode::ACTIVITYPUB); diff --git a/src/Protocol/OStatus.php b/src/Protocol/OStatus.php index 5e34636b21..2a160724b4 100644 --- a/src/Protocol/OStatus.php +++ b/src/Protocol/OStatus.php @@ -976,44 +976,6 @@ class OStatus } } - /** - * Cleans the body of a post if it contains picture links - * - * @param string $body The body - * @param integer $uriId - * @return string The cleaned body - * @throws \Friendica\Network\HTTPException\InternalServerErrorException - */ - public static function formatPicturePost(string $body, int $uriid): string - { - $siteinfo = BBCode::getAttachedData($body); - - if (($siteinfo['type'] == 'photo') && (!empty($siteinfo['preview']) || !empty($siteinfo['image']))) { - if (isset($siteinfo['preview'])) { - $preview = $siteinfo['preview']; - } else { - $preview = $siteinfo['image']; - } - - // Is it a remote picture? Then make a smaller preview here - $preview = Post\Link::getByLink($uriid, $preview, Proxy::SIZE_SMALL); - - // Is it a local picture? Then make it smaller here - $preview = str_replace(['-0.jpg', '-0.png'], ['-2.jpg', '-2.png'], $preview); - $preview = str_replace(['-1.jpg', '-1.png'], ['-2.jpg', '-2.png'], $preview); - - if (isset($siteinfo['url'])) { - $url = $siteinfo['url']; - } else { - $url = $siteinfo['image']; - } - - $body = trim($siteinfo['text']) . ' [url]' . $url . "[/url]\n[img]" . $preview . '[/img]'; - } - - return $body; - } - /** * Adds the header elements to the XML document * @@ -1140,51 +1102,7 @@ class OStatus */ public static function getAttachment(DOMDocument $doc, DOMElement $root, array $item) { - $siteinfo = BBCode::getAttachedData($item['body']); - - switch ($siteinfo['type']) { - case 'photo': - if (!empty($siteinfo['image'])) { - $imgdata = Images::getInfoFromURLCached($siteinfo['image']); - if ($imgdata) { - $attributes = [ - 'rel' => 'enclosure', - 'href' => $siteinfo['image'], - 'type' => $imgdata['mime'], - 'length' => intval($imgdata['size']), - ]; - XML::addElement($doc, $root, 'link', '', $attributes); - } - } - break; - - case 'video': - $attributes = [ - 'rel' => 'enclosure', - 'href' => $siteinfo['url'], - 'type' => 'text/html; charset=UTF-8', - 'length' => '0', - 'title' => ($siteinfo['title'] ?? '') ?: $siteinfo['url'], - ]; - XML::addElement($doc, $root, 'link', '', $attributes); - break; - } - - if (!DI::config()->get('system', 'ostatus_not_attach_preview') && ($siteinfo['type'] != 'photo') && isset($siteinfo['image'])) { - $imgdata = Images::getInfoFromURLCached($siteinfo['image']); - if ($imgdata) { - $attributes = [ - 'rel' => 'enclosure', - 'href' => $siteinfo['image'], - 'type' => $imgdata['mime'], - 'length' => intval($imgdata['size']), - ]; - - XML::addElement($doc, $root, 'link', '', $attributes); - } - } - - foreach (Post\Media::getByURIId($item['uri-id'], [Post\Media::DOCUMENT, Post\Media::TORRENT]) as $attachment) { + foreach (Post\Media::getByURIId($item['uri-id'], [Post\Media::AUDIO, Post\Media::IMAGE, Post\Media::VIDEO, Post\Media::DOCUMENT, Post\Media::TORRENT]) as $attachment) { $attributes = ['rel' => 'enclosure', 'href' => $attachment['url'], 'type' => $attachment['mimetype']]; @@ -1597,7 +1515,6 @@ class OStatus XML::addElement($doc, $entry, 'title', html_entity_decode($title, ENT_QUOTES, 'UTF-8')); $body = Post\Media::addAttachmentsToBody($item['uri-id'], DI::contentItem()->addSharedPost($item)); - $body = self::formatPicturePost($body, $item['uri-id']); if (!empty($item['title'])) { $body = '[b]' . $item['title'] . "[/b]\n\n" . $body; diff --git a/src/Worker/Notifier.php b/src/Worker/Notifier.php index 473721fb7b..e4189c2c75 100644 --- a/src/Worker/Notifier.php +++ b/src/Worker/Notifier.php @@ -517,7 +517,11 @@ class Notifier foreach ($contacts as $contact) { // Direct delivery of local contacts - if (!in_array($cmd, [Delivery::RELOCATION, Delivery::SUGGESTION, Delivery::DELETION, Delivery::MAIL]) && $target_uid = User::getIdForURL($contact['url'])) { + if (!in_array($cmd, [Delivery::RELOCATION, Delivery::SUGGESTION, Delivery::MAIL]) && $target_uid = User::getIdForURL($contact['url'])) { + if ($cmd == Delivery::DELETION) { + Logger::info('No need to deliver deletions internally', ['uid' => $target_uid, 'guid' => $target_item['guid'], 'uri-id' => $target_item['uri-id'], 'uri' => $target_item['uri']]); + continue; + } if ($target_item['origin'] || ($target_item['network'] != Protocol::ACTIVITYPUB)) { if ($target_uid != $target_item['uid']) { $fields = ['protocol' => Conversation::PARCEL_LOCAL_DFRN, 'direction' => Conversation::PUSH, 'post-reason' => Item::PR_DIRECT]; @@ -839,7 +843,11 @@ class Notifier if ((count($receivers) == 1) && Network::isLocalLink($inbox)) { $contact = Contact::getById($receivers[0], ['url']); - if (!in_array($cmd, [Delivery::RELOCATION, Delivery::SUGGESTION, Delivery::DELETION, Delivery::MAIL]) && ($target_uid = User::getIdForURL($contact['url']))) { + if (!in_array($cmd, [Delivery::RELOCATION, Delivery::SUGGESTION, Delivery::MAIL]) && ($target_uid = User::getIdForURL($contact['url']))) { + if ($cmd == Delivery::DELETION) { + Logger::info('No need to deliver deletions internally', ['uid' => $target_uid, 'guid' => $target_item['guid'], 'uri-id' => $target_item['uri-id'], 'uri' => $target_item['uri']]); + continue; + } if ($target_item['origin'] || ($target_item['network'] != Protocol::ACTIVITYPUB)) { if ($target_uid != $target_item['uid']) { $fields = ['protocol' => Conversation::PARCEL_LOCAL_DFRN, 'direction' => Conversation::PUSH, 'post-reason' => Item::PR_BCC]; diff --git a/src/Worker/OnePoll.php b/src/Worker/OnePoll.php index cb892d3326..bf03e2cf03 100644 --- a/src/Worker/OnePoll.php +++ b/src/Worker/OnePoll.php @@ -38,6 +38,7 @@ use Friendica\Protocol\ActivityPub; use Friendica\Protocol\Email; use Friendica\Protocol\Feed; use Friendica\Util\DateTimeFormat; +use Friendica\Util\Network; use Friendica\Util\Strings; class OnePoll @@ -157,6 +158,11 @@ class OnePoll return false; } + if (!Network::isValidHttpUrl($contact['poll'])) { + Logger::notice('Poll address is not valid', ['id' => $contact['id'], 'uid' => $contact['uid'], 'url' => $contact['url'], 'poll' => $contact['poll']]); + return false; + } + $cookiejar = tempnam(System::getTempPath(), 'cookiejar-onepoll-'); $curlResult = DI::httpClient()->get($contact['poll'], HttpClientAccept::FEED_XML, [HttpClientOptions::COOKIEJAR => $cookiejar]); unlink($cookiejar); From 94b63e6a00d288f1d4f2ef4560d2768d97d434bc Mon Sep 17 00:00:00 2001 From: Michael Vogel Date: Fri, 27 Jan 2023 07:21:08 +0100 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Hypolite Petovan --- src/Module/Photo.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Module/Photo.php b/src/Module/Photo.php index 7f3429b028..bb7ee60297 100644 --- a/src/Module/Photo.php +++ b/src/Module/Photo.php @@ -353,7 +353,6 @@ class Photo extends BaseModule // If it is a local link, we save resources by just redirecting to it. if (Network::isLocalLink($url)) { System::externalRedirect($url); - System::exit(); } $mimetext = ''; @@ -402,7 +401,6 @@ class Photo extends BaseModule } if (Network::isLocalLink($url)) { System::externalRedirect($url); - System::exit(); } } return MPhoto::createPhotoForExternalResource($url, 0, $mimetext, $contact['blurhash'] ?? null, $customsize, $customsize); @@ -430,7 +428,6 @@ class Photo extends BaseModule $url = Contact::getDefaultHeader($contact); if (Network::isLocalLink($url)) { System::externalRedirect($url); - System::exit(); } } return MPhoto::createPhotoForExternalResource($url); @@ -467,7 +464,6 @@ class Photo extends BaseModule if (Network::isLocalLink($default)) { System::externalRedirect($default); - System::exit(); } $parts = parse_url($default);