From 5c5d7eb04fbacbe5987bd83022b158e095d13f13 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Thu, 22 Feb 2024 00:53:52 -0500 Subject: [PATCH 1/4] Fix several vulnerabilities (#13927) * Escape HTML in the location field of a calendar event post - This allowed script tags to be interpreted in the post display of an event. * Add form security token check to /admin/phpinfo module - This prevents basic XSS attacks against /admin/phpinfo * Add form security token check to /babel module - This prevents basic XSS attacks against /babel * Prevent pass-through for attachments - This addresses a straightforward Reflected XSS vulnerability if a malicious HTML/Javascript file is attached to a post through upload * Prevent overwriting cid on event edit - This allowed to share an event as any other user after zeroing the cid field of an existing event --- src/Model/Event.php | 7 ++--- src/Module/Admin/PhpInfo.php | 2 ++ src/Module/Attach.php | 6 +--- src/Module/BaseAdmin.php | 2 +- src/Module/Calendar/Event/API.php | 3 +- src/Module/Debug/Babel.php | 28 ++++++++++--------- view/templates/babel.tpl | 3 +- .../frio/templates/event_stream_item.tpl | 2 +- 8 files changed, 26 insertions(+), 27 deletions(-) diff --git a/src/Model/Event.php b/src/Model/Event.php index 3709af06c1..1866303783 100644 --- a/src/Model/Event.php +++ b/src/Model/Event.php @@ -925,9 +925,6 @@ class Event $end_short = ''; } - // Format the event location. - $location = self::locationToArray($item['event-location']); - // Construct the profile link (magic-auth). $author = [ 'uid' => 0, @@ -964,7 +961,7 @@ class Event '$show_map_label' => DI::l10n()->t('Show map'), '$hide_map_label' => DI::l10n()->t('Hide map'), '$map_btn_label' => DI::l10n()->t('Show map'), - '$location' => $location + '$location' => self::locationToTemplateVars($item['event-location']), ]); return $return; @@ -984,7 +981,7 @@ class Event * 'coordinates' => Latitude and longitude (e.g. '48.864716,2.349014').
* @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - private static function locationToArray(string $s = ''): array + private static function locationToTemplateVars(string $s = ''): array { if ($s == '') { return []; diff --git a/src/Module/Admin/PhpInfo.php b/src/Module/Admin/PhpInfo.php index ed760d226c..79162ebd36 100644 --- a/src/Module/Admin/PhpInfo.php +++ b/src/Module/Admin/PhpInfo.php @@ -30,6 +30,8 @@ class PhpInfo extends BaseAdmin { self::checkAdminAccess(); + self::checkFormSecurityTokenForbiddenOnError('phpinfo', 't'); + phpinfo(); System::exit(); } diff --git a/src/Module/Attach.php b/src/Module/Attach.php index 7d877c1265..a339b381c5 100644 --- a/src/Module/Attach.php +++ b/src/Module/Attach.php @@ -65,11 +65,7 @@ class Attach extends BaseModule // error in Chrome for filenames with commas in them header('Content-type: ' . $item['filetype']); header('Content-length: ' . $item['filesize']); - if (isset($_GET['attachment']) && $_GET['attachment'] === '0') { - header('Content-disposition: filename="' . $item['filename'] . '"'); - } else { - header('Content-disposition: attachment; filename="' . $item['filename'] . '"'); - } + header('Content-disposition: attachment; filename="' . $item['filename'] . '"'); echo $data; System::exit(); diff --git a/src/Module/BaseAdmin.php b/src/Module/BaseAdmin.php index 0a5f5fa4c8..bdcead545f 100644 --- a/src/Module/BaseAdmin.php +++ b/src/Module/BaseAdmin.php @@ -104,7 +104,7 @@ abstract class BaseAdmin extends BaseModule 'logsview' => ['admin/logs/view' , DI::l10n()->t('View Logs') , 'viewlogs'], ]], 'diagnostics' => [DI::l10n()->t('Diagnostics'), [ - 'phpinfo' => ['admin/phpinfo' , DI::l10n()->t('PHP Info') , 'phpinfo'], + 'phpinfo' => ['admin/phpinfo?t=' . self::getFormSecurityToken('phpinfo'), DI::l10n()->t('PHP Info') , 'phpinfo'], 'probe' => ['probe' , DI::l10n()->t('probe address') , 'probe'], 'webfinger' => ['webfinger' , DI::l10n()->t('check webfinger') , 'webfinger'], 'babel' => ['babel' , DI::l10n()->t('Babel') , 'babel'], diff --git a/src/Module/Calendar/Event/API.php b/src/Module/Calendar/Event/API.php index 9deb14a647..2539bf3592 100644 --- a/src/Module/Calendar/Event/API.php +++ b/src/Module/Calendar/Event/API.php @@ -142,7 +142,8 @@ class API extends BaseModule { $eventId = !empty($request['event_id']) ? intval($request['event_id']) : 0; $uid = (int)$this->session->getLocalUserId(); - $cid = !empty($request['cid']) ? intval($request['cid']) : 0; + // No overwriting event.cid on edit + $cid = !empty($request['cid']) && !$eventId ? intval($request['cid']) : 0; $strStartDateTime = Strings::escapeHtml($request['start_text'] ?? ''); $strFinishDateTime = Strings::escapeHtml($request['finish_text'] ?? ''); diff --git a/src/Module/Debug/Babel.php b/src/Module/Debug/Babel.php index 0b7b1d779e..a67d522961 100644 --- a/src/Module/Debug/Babel.php +++ b/src/Module/Debug/Babel.php @@ -43,10 +43,11 @@ class Babel extends BaseModule } $results = []; - if (!empty($_REQUEST['text'])) { - switch (($_REQUEST['type'] ?? '') ?: 'bbcode') { + if (!empty($request['text'])) { + self::checkFormSecurityTokenForbiddenOnError('babel'); + switch (($request['type'] ?? '') ?: 'bbcode') { case 'bbcode': - $bbcode = $_REQUEST['text']; + $bbcode = $request['text']; $results[] = [ 'title' => DI::l10n()->t('Source input'), 'content' => visible_whitespace($bbcode) @@ -136,7 +137,7 @@ class Babel extends BaseModule ]; break; case 'diaspora': - $diaspora = trim($_REQUEST['text']); + $diaspora = trim($request['text']); $results[] = [ 'title' => DI::l10n()->t('Source input (Diaspora format)'), 'content' => visible_whitespace($diaspora), @@ -144,7 +145,7 @@ class Babel extends BaseModule $markdown = XML::unescape($diaspora); case 'markdown': - $markdown = $markdown ?? trim($_REQUEST['text']); + $markdown = $markdown ?? trim($request['text']); $results[] = [ 'title' => DI::l10n()->t('Source input (Markdown)'), @@ -169,7 +170,7 @@ class Babel extends BaseModule ]; break; case 'html' : - $html = trim($_REQUEST['text']); + $html = trim($request['text']); $results[] = [ 'title' => DI::l10n()->t('Raw HTML input'), 'content' => visible_whitespace($html), @@ -239,7 +240,7 @@ class Babel extends BaseModule ]; break; case 'twitter': - $json = trim($_REQUEST['text']); + $json = trim($request['text']); if (file_exists('addon/twitter/twitter.php')) { require_once 'addon/twitter/twitter.php'; @@ -302,13 +303,14 @@ class Babel extends BaseModule $tpl = Renderer::getMarkupTemplate('babel.tpl'); $o = Renderer::replaceMacros($tpl, [ '$title' => DI::l10n()->t('Babel Diagnostic'), - '$text' => ['text', DI::l10n()->t('Source text'), $_REQUEST['text'] ?? '', ''], - '$type_bbcode' => ['type', DI::l10n()->t('BBCode'), 'bbcode', '', (($_REQUEST['type'] ?? '') ?: 'bbcode') == 'bbcode'], - '$type_diaspora' => ['type', DI::l10n()->t('Diaspora'), 'diaspora', '', (($_REQUEST['type'] ?? '') ?: 'bbcode') == 'diaspora'], - '$type_markdown' => ['type', DI::l10n()->t('Markdown'), 'markdown', '', (($_REQUEST['type'] ?? '') ?: 'bbcode') == 'markdown'], - '$type_html' => ['type', DI::l10n()->t('HTML'), 'html', '', (($_REQUEST['type'] ?? '') ?: 'bbcode') == 'html'], + '$form_security_token' => self::getFormSecurityToken('babel'), + '$text' => ['text', DI::l10n()->t('Source text'), $request['text'] ?? '', ''], + '$type_bbcode' => ['type', DI::l10n()->t('BBCode'), 'bbcode', '', (($request['type'] ?? '') ?: 'bbcode') == 'bbcode'], + '$type_diaspora' => ['type', DI::l10n()->t('Diaspora'), 'diaspora', '', (($request['type'] ?? '') ?: 'bbcode') == 'diaspora'], + '$type_markdown' => ['type', DI::l10n()->t('Markdown'), 'markdown', '', (($request['type'] ?? '') ?: 'bbcode') == 'markdown'], + '$type_html' => ['type', DI::l10n()->t('HTML'), 'html', '', (($request['type'] ?? '') ?: 'bbcode') == 'html'], '$flag_twitter' => file_exists('addon/twitter/twitter.php'), - '$type_twitter' => ['type', DI::l10n()->t('Twitter Source / Tweet URL (requires API key)'), 'twitter', '', (($_REQUEST['type'] ?? '') ?: 'bbcode') == 'twitter'], + '$type_twitter' => ['type', DI::l10n()->t('Twitter Source / Tweet URL (requires API key)'), 'twitter', '', (($request['type'] ?? '') ?: 'bbcode') == 'twitter'], '$results' => $results, '$submit' => DI::l10n()->t('Submit'), ]); diff --git a/view/templates/babel.tpl b/view/templates/babel.tpl index 4e8e12d5c2..ee002024b5 100644 --- a/view/templates/babel.tpl +++ b/view/templates/babel.tpl @@ -1,6 +1,7 @@

{{$title}}

+
{{include file="field_textarea.tpl" field=$text}} @@ -30,4 +31,4 @@ {{/foreach}}
-{{/if}} \ No newline at end of file +{{/if}} diff --git a/view/theme/frio/templates/event_stream_item.tpl b/view/theme/frio/templates/event_stream_item.tpl index 2f2af2732e..74a9734907 100644 --- a/view/theme/frio/templates/event_stream_item.tpl +++ b/view/theme/frio/templates/event_stream_item.tpl @@ -23,7 +23,7 @@ {{if $location.name}} - {{$location.name nofilter}} + {{$location.name}} {{/if}}
From e16b6ee6e16d06025f8ea17ca6e130f3484ff80b Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Thu, 22 Feb 2024 15:08:32 -0500 Subject: [PATCH 2/4] Check form security token in /settings/userexport module (#13929) * Escape HTML in the location field of a calendar event post - This allowed script tags to be interpreted in the post display of an event. * Add form security token check to /admin/phpinfo module - This prevents basic XSS attacks against /admin/phpinfo * Add form security token check to /babel module - This prevents basic XSS attacks against /babel * Prevent pass-through for attachments - This addresses a straightforward Reflected XSS vulnerability if a malicious HTML/Javascript file is attached to a post through upload * Prevent overwriting cid on event edit - This allowed to share an event as any other user after zeroing the cid field of an existing event * Check form security token in /settings/userexport module - Prevents basic XSS attacks against /settings/userexport/* --- src/Module/Settings/UserExport.php | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/Module/Settings/UserExport.php b/src/Module/Settings/UserExport.php index f0be187513..6473b1ad61 100644 --- a/src/Module/Settings/UserExport.php +++ b/src/Module/Settings/UserExport.php @@ -29,7 +29,6 @@ use Friendica\Core\Session\Capability\IHandleUserSessions; use Friendica\Core\System; use Friendica\Database\DBA; use Friendica\Database\Definition\DbaDefinition; -use Friendica\DI; use Friendica\Model\Contact; use Friendica\Model\Item; use Friendica\Model\Post; @@ -47,8 +46,7 @@ use Psr\Log\LoggerInterface; **/ class UserExport extends BaseSettings { - /** @var DbaDefinition */ - private $dbaDefinition; + private DbaDefinition $dbaDefinition; public function __construct(DbaDefinition $dbaDefinition, IHandleUserSessions $session, App\Page $page, L10n $l10n, App\BaseURL $baseUrl, App\Arguments $args, LoggerInterface $logger, Profiler $profiler, Response $response, array $server, array $parameters = []) { @@ -86,10 +84,12 @@ class UserExport extends BaseSettings * options shown on "Export personal data" page * list of array( 'link url', 'link text', 'help text' ) */ + + $t = self::getFormSecurityToken('userexport'); $options = [ - ['settings/userexport/account', $this->l10n->t('Export account'), $this->l10n->t('Export your account info and contacts. Use this to make a backup of your account and/or to move it to another server.')], - ['settings/userexport/backup', $this->l10n->t('Export all'), $this->l10n->t('Export your account info, contacts and all your items as json. Could be a very big file, and could take a lot of time. Use this to make a full backup of your account (photos are not exported)')], - ['settings/userexport/contact', $this->l10n->t('Export Contacts to CSV'), $this->l10n->t('Export the list of the accounts you are following as CSV file. Compatible to e.g. Mastodon.')], + ['settings/userexport/account?t=' . $t, $this->l10n->t('Export account'), $this->l10n->t('Export your account info and contacts. Use this to make a backup of your account and/or to move it to another server.')], + ['settings/userexport/backup?t=' . $t, $this->l10n->t('Export all'), $this->l10n->t('Export your account info, contacts and all your items as json. Could be a very big file, and could take a lot of time. Use this to make a full backup of your account (photos are not exported)')], + ['settings/userexport/contact?t=' . $t, $this->l10n->t('Export Contacts to CSV'), $this->l10n->t('Export the list of the accounts you are following as CSV file. Compatible to e.g. Mastodon.')], ]; Hook::callAll('uexport_options', $options); @@ -115,20 +115,21 @@ class UserExport extends BaseSettings } if (isset($this->parameters['action'])) { + self::checkFormSecurityTokenForbiddenOnError('userexport', 't'); switch ($this->parameters['action']) { case 'backup': header('Content-type: application/json'); - header('Content-Disposition: attachment; filename="' . DI::app()->getLoggedInUserNickname() . '.' . $this->parameters['action'] . '"'); + header('Content-Disposition: attachment; filename="' . $this->session->getLocalUserNickname() . '.' . $this->parameters['action'] . '"'); $this->echoAll($this->session->getLocalUserId()); break; case 'account': header('Content-type: application/json'); - header('Content-Disposition: attachment; filename="' . DI::app()->getLoggedInUserNickname() . '.' . $this->parameters['action'] . '"'); + header('Content-Disposition: attachment; filename="' . $this->session->getLocalUserNickname() . '.' . $this->parameters['action'] . '"'); $this->echoAccount($this->session->getLocalUserId()); break; case 'contact': header('Content-type: application/csv'); - header('Content-Disposition: attachment; filename="' . DI::app()->getLoggedInUserNickname() . '-contacts.csv' . '"'); + header('Content-Disposition: attachment; filename="' . $this->session->getLocalUserNickname() . '-contacts.csv' . '"'); $this->echoContactsAsCSV($this->session->getLocalUserId()); break; } @@ -156,11 +157,8 @@ class UserExport extends BaseSettings if (!isset($row[$column])) { continue; } - if ($field['type'] == 'datetime') { - $p[$column] = $row[$column] ?? DBA::NULL_DATETIME; - } else { - $p[$column] = $row[$column]; - } + + $p[$column] = $row[$column]; } $result[] = $p; } From a25dbf839aebd9065c25761060d80128e03eb9fc Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Thu, 22 Feb 2024 22:57:20 -0500 Subject: [PATCH 3/4] Remove photo user id fallback from 2021 - Remove deprecated /photos/{nickname} fallback routes - The contact id fallback is a lie, there's no replacement feature --- src/Module/Photo.php | 7 ------- static/routes.config.php | 9 --------- 2 files changed, 16 deletions(-) diff --git a/src/Module/Photo.php b/src/Module/Photo.php index 1bac70cdfd..6bf81ac7cb 100644 --- a/src/Module/Photo.php +++ b/src/Module/Photo.php @@ -100,7 +100,6 @@ class Photo extends BaseApi $id = $account['id']; } - // Contact Id Fallback, to remove after version 2021.12 if (isset($this->parameters['contact_id'])) { $id = intval($this->parameters['contact_id']); } @@ -115,12 +114,6 @@ class Photo extends BaseApi $id = $user['uid']; } - // User Id Fallback, to remove after version 2021.12 - if (!empty($this->parameters['uid_ext'])) { - $id = intval(pathinfo($this->parameters['uid_ext'], PATHINFO_FILENAME)); - } - - // Please refactor this for the love of everything that's good if (isset($this->parameters['id'])) { $id = $this->parameters['id']; } diff --git a/static/routes.config.php b/static/routes.config.php index 67ffec8e2b..146255adc6 100644 --- a/static/routes.config.php +++ b/static/routes.config.php @@ -575,22 +575,13 @@ return [ '/{name}' => [Module\Photo::class, [R::GET]], '/{type}/{id:\d+}' => [Module\Photo::class, [R::GET]], '/{type:contact|header}/{guid}' => [Module\Photo::class, [R::GET]], - // User Id Fallback, to remove after version 2021.12 - '/{type}/{uid_ext:\d+\..*}' => [Module\Photo::class, [R::GET]], '/{type}/{nickname_ext}' => [Module\Photo::class, [R::GET]], - // Contact Id Fallback, to remove after version 2021.12 '/{type:contact|header}/{customsize:\d+}/{contact_id:\d+}' => [Module\Photo::class, [R::GET]], '/{type:contact|header}/{customsize:\d+}/{guid}' => [Module\Photo::class, [R::GET]], '/{type}/{customsize:\d+}/{id:\d+}' => [Module\Photo::class, [R::GET]], - // User Id Fallback, to remove after version 2021.12 - '/{type}/{customsize:\d+}/{uid_ext:\d+\..*}' => [Module\Photo::class, [R::GET]], '/{type}/{customsize:\d+}/{nickname_ext}' => [Module\Photo::class, [R::GET]], ], - // Kept for backwards-compatibility - // @TODO remove by version 2023.12 - '/photos/{nickname}' => [Module\Profile\Photos::class, [R::GET]], - '/ping' => [Module\Notifications\Ping::class, [R::GET]], '/post' => [ From 0a73050de1c3423380636af0e12081f20420db96 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Thu, 22 Feb 2024 22:57:57 -0500 Subject: [PATCH 4/4] Increase API photo preview size for Mastodon API to 640 --- src/Factory/Api/Mastodon/Attachment.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Factory/Api/Mastodon/Attachment.php b/src/Factory/Api/Mastodon/Attachment.php index 727b77630a..18207fa1be 100644 --- a/src/Factory/Api/Mastodon/Attachment.php +++ b/src/Factory/Api/Mastodon/Attachment.php @@ -95,12 +95,12 @@ class Attachment extends BaseFactory $remote = $attachment['url']; if ($type == 'image') { $url = Post\Media::getPreviewUrlForId($attachment['id']); - $preview = Post\Media::getPreviewUrlForId($attachment['id'], Proxy::SIZE_SMALL); + $preview = Post\Media::getPreviewUrlForId($attachment['id'], Proxy::SIZE_MEDIUM); } else { $url = $attachment['url']; if (!empty($attachment['preview'])) { - $preview = Post\Media::getPreviewUrlForId($attachment['id'], Proxy::SIZE_SMALL); + $preview = Post\Media::getPreviewUrlForId($attachment['id'], Proxy::SIZE_MEDIUM); } else { $preview = ''; }