Skip to content

Commit

Permalink
Fix several vulnerabilities (#13927)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
MrPetovan committed Feb 22, 2024
1 parent fc3898f commit 5c5d7eb
Show file tree
Hide file tree
Showing 8 changed files with 26 additions and 27 deletions.
7 changes: 2 additions & 5 deletions src/Model/Event.php
Expand Up @@ -925,9 +925,6 @@ public static function getItemHTML(array $item): string
$end_short = '';
}

// Format the event location.
$location = self::locationToArray($item['event-location']);

// Construct the profile link (magic-auth).
$author = [
'uid' => 0,
Expand Down Expand Up @@ -964,7 +961,7 @@ public static function getItemHTML(array $item): string
'$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;
Expand All @@ -984,7 +981,7 @@ public static function getItemHTML(array $item): string
* 'coordinates' => Latitude and longitude (e.g. '48.864716,2.349014').<br>
* @throws \Friendica\Network\HTTPException\InternalServerErrorException
*/
private static function locationToArray(string $s = ''): array
private static function locationToTemplateVars(string $s = ''): array
{
if ($s == '') {
return [];
Expand Down
2 changes: 2 additions & 0 deletions src/Module/Admin/PhpInfo.php
Expand Up @@ -30,6 +30,8 @@ protected function rawContent(array $request = [])
{
self::checkAdminAccess();

self::checkFormSecurityTokenForbiddenOnError('phpinfo', 't');

phpinfo();
System::exit();
}
Expand Down
6 changes: 1 addition & 5 deletions src/Module/Attach.php
Expand Up @@ -65,11 +65,7 @@ protected function rawContent(array $request = [])
// 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();
Expand Down
2 changes: 1 addition & 1 deletion src/Module/BaseAdmin.php
Expand Up @@ -104,7 +104,7 @@ protected function content(array $request = []): string
'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'],
Expand Down
3 changes: 2 additions & 1 deletion src/Module/Calendar/Event/API.php
Expand Up @@ -142,7 +142,8 @@ protected function createEvent(array $request)
{
$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'] ?? '');
Expand Down
28 changes: 15 additions & 13 deletions src/Module/Debug/Babel.php
Expand Up @@ -43,10 +43,11 @@ function visible_whitespace($s)
}

$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)
Expand Down Expand Up @@ -136,15 +137,15 @@ function visible_whitespace($s)
];
break;
case 'diaspora':
$diaspora = trim($_REQUEST['text']);
$diaspora = trim($request['text']);
$results[] = [
'title' => DI::l10n()->t('Source input (Diaspora format)'),
'content' => visible_whitespace($diaspora),
];

$markdown = XML::unescape($diaspora);
case 'markdown':
$markdown = $markdown ?? trim($_REQUEST['text']);
$markdown = $markdown ?? trim($request['text']);

$results[] = [
'title' => DI::l10n()->t('Source input (Markdown)'),
Expand All @@ -169,7 +170,7 @@ function visible_whitespace($s)
];
break;
case 'html' :
$html = trim($_REQUEST['text']);
$html = trim($request['text']);
$results[] = [
'title' => DI::l10n()->t('Raw HTML input'),
'content' => visible_whitespace($html),
Expand Down Expand Up @@ -239,7 +240,7 @@ function visible_whitespace($s)
];
break;
case 'twitter':
$json = trim($_REQUEST['text']);
$json = trim($request['text']);

if (file_exists('addon/twitter/twitter.php')) {
require_once 'addon/twitter/twitter.php';
Expand Down Expand Up @@ -302,13 +303,14 @@ function visible_whitespace($s)
$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'),
]);
Expand Down
3 changes: 2 additions & 1 deletion view/templates/babel.tpl
@@ -1,6 +1,7 @@
<div id="babel" class="generic-page-wrapper">
<h2>{{$title}}</h2>
<form action="babel" method="post" class="panel panel-default">
<input type="hidden" name="form_security_token" value="{{$form_security_token}}">
<div class="panel-body">
<div class="form-group">
{{include file="field_textarea.tpl" field=$text}}
Expand Down Expand Up @@ -30,4 +31,4 @@
{{/foreach}}
</div>
</div>
{{/if}}
{{/if}}
2 changes: 1 addition & 1 deletion view/theme/frio/templates/event_stream_item.tpl
Expand Up @@ -23,7 +23,7 @@
</span>
{{if $location.name}}
<span role="presentation" aria-hidden="true"> · </span>
<span class="event-location event-card-location">{{$location.name nofilter}}</span>
<span class="event-location event-card-location">{{$location.name}}</span>
{{/if}}
</div>
<div class="event-card-profile-name profile-entry-name">
Expand Down

0 comments on commit 5c5d7eb

Please sign in to comment.