From 5c5d7eb04fbacbe5987bd83022b158e095d13f13 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Thu, 22 Feb 2024 00:53:52 -0500 Subject: [PATCH] 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}}