Security: fix multiple XSS vulnerabilities + fix search tags with special chars

XSS vulnerabilities fixed in editlink, linklist, tag.cloud and tag.list.

Also fixed tag search with special characters: urlencode function needs to be applied on raw data, before espaping, otherwise the rendered URL is wrong.
This commit is contained in:
ArthurHoaro 2020-10-06 17:30:18 +02:00
parent df25b28dcd
commit 72fbbcd679
11 changed files with 68 additions and 27 deletions

View file

@ -95,14 +95,14 @@ function escape($input)
return null; return null;
} }
if (is_bool($input)) { if (is_bool($input) || is_int($input) || is_float($input) || $input instanceof DateTimeInterface) {
return $input; return $input;
} }
if (is_array($input)) { if (is_array($input)) {
$out = array(); $out = array();
foreach ($input as $key => $value) { foreach ($input as $key => $value) {
$out[$key] = escape($value); $out[escape($key)] = escape($value);
} }
return $out; return $out;
} }

View file

@ -58,7 +58,9 @@ abstract class BookmarkFormatter
$out['title'] = $this->formatTitle($bookmark); $out['title'] = $this->formatTitle($bookmark);
$out['description'] = $this->formatDescription($bookmark); $out['description'] = $this->formatDescription($bookmark);
$out['thumbnail'] = $this->formatThumbnail($bookmark); $out['thumbnail'] = $this->formatThumbnail($bookmark);
$out['urlencoded_taglist'] = $this->formatUrlEncodedTagList($bookmark);
$out['taglist'] = $this->formatTagList($bookmark); $out['taglist'] = $this->formatTagList($bookmark);
$out['urlencoded_tags'] = $this->formatUrlEncodedTagString($bookmark);
$out['tags'] = $this->formatTagString($bookmark); $out['tags'] = $this->formatTagString($bookmark);
$out['sticky'] = $bookmark->isSticky(); $out['sticky'] = $bookmark->isSticky();
$out['private'] = $bookmark->isPrivate(); $out['private'] = $bookmark->isPrivate();
@ -181,6 +183,18 @@ abstract class BookmarkFormatter
return $this->filterTagList($bookmark->getTags()); return $this->filterTagList($bookmark->getTags());
} }
/**
* Format Url Encoded Tags
*
* @param Bookmark $bookmark instance
*
* @return array formatted Tags
*/
protected function formatUrlEncodedTagList($bookmark)
{
return array_map('urlencode', $this->filterTagList($bookmark->getTags()));
}
/** /**
* Format TagString * Format TagString
* *
@ -193,6 +207,18 @@ abstract class BookmarkFormatter
return implode(' ', $this->formatTagList($bookmark)); return implode(' ', $this->formatTagList($bookmark));
} }
/**
* Format TagString
*
* @param Bookmark $bookmark instance
*
* @return string formatted TagString
*/
protected function formatUrlEncodedTagString($bookmark)
{
return implode(' ', $this->formatUrlEncodedTagList($bookmark));
}
/** /**
* Format Class * Format Class
* Used to add specific CSS class for a link * Used to add specific CSS class for a link

View file

@ -78,13 +78,13 @@ class ManageShaareController extends ShaarliAdminController
$title = $this->container->conf->get('general.default_note_title', t('Note: ')); $title = $this->container->conf->get('general.default_note_title', t('Note: '));
} }
$link = escape([ $link = [
'title' => $title, 'title' => $title,
'url' => $url ?? '', 'url' => $url ?? '',
'description' => $description ?? '', 'description' => $description ?? '',
'tags' => $tags ?? '', 'tags' => $tags ?? '',
'private' => $private, 'private' => $private,
]); ];
} else { } else {
$formatter = $this->container->formatterFactory->getFormatter('raw'); $formatter = $this->container->formatterFactory->getFormatter('raw');
$link = $formatter->format($bookmark); $link = $formatter->format($bookmark);
@ -345,14 +345,14 @@ class ManageShaareController extends ShaarliAdminController
$tags[BookmarkMarkdownFormatter::NO_MD_TAG] = 1; $tags[BookmarkMarkdownFormatter::NO_MD_TAG] = 1;
} }
$data = [ $data = escape([
'link' => $link, 'link' => $link,
'link_is_new' => $isNew, 'link_is_new' => $isNew,
'http_referer' => escape($this->container->environment['HTTP_REFERER'] ?? ''), 'http_referer' => $this->container->environment['HTTP_REFERER'] ?? '',
'source' => $request->getParam('source') ?? '', 'source' => $request->getParam('source') ?? '',
'tags' => $tags, 'tags' => $tags,
'default_private_links' => $this->container->conf->get('privacy.default_private_links', false), 'default_private_links' => $this->container->conf->get('privacy.default_private_links', false),
]; ]);
$this->executePageHooks('render_editlink', $data, TemplatePage::EDIT_LINK); $this->executePageHooks('render_editlink', $data, TemplatePage::EDIT_LINK);

View file

@ -41,8 +41,8 @@ class ManageTagController extends ShaarliAdminController
$isDelete = null !== $request->getParam('deletetag') && null === $request->getParam('renametag'); $isDelete = null !== $request->getParam('deletetag') && null === $request->getParam('renametag');
$fromTag = escape(trim($request->getParam('fromtag') ?? '')); $fromTag = trim($request->getParam('fromtag') ?? '');
$toTag = escape(trim($request->getParam('totag') ?? '')); $toTag = trim($request->getParam('totag') ?? '');
if (0 === strlen($fromTag) || false === $isDelete && 0 === strlen($toTag)) { if (0 === strlen($fromTag) || false === $isDelete && 0 === strlen($toTag)) {
$this->saveWarningMessage(t('Invalid tags provided.')); $this->saveWarningMessage(t('Invalid tags provided.'));

View file

@ -34,7 +34,7 @@ class BookmarkListController extends ShaarliVisitorController
$formatter = $this->container->formatterFactory->getFormatter(); $formatter = $this->container->formatterFactory->getFormatter();
$formatter->addContextData('base_path', $this->container->basePath); $formatter->addContextData('base_path', $this->container->basePath);
$searchTags = escape(normalize_spaces($request->getParam('searchtags') ?? '')); $searchTags = normalize_spaces($request->getParam('searchtags') ?? '');
$searchTerm = escape(normalize_spaces($request->getParam('searchterm') ?? ''));; $searchTerm = escape(normalize_spaces($request->getParam('searchterm') ?? ''));;
// Filter bookmarks according search parameters. // Filter bookmarks according search parameters.
@ -104,8 +104,9 @@ class BookmarkListController extends ShaarliVisitorController
'page_current' => $page, 'page_current' => $page,
'page_max' => $pageCount, 'page_max' => $pageCount,
'result_count' => count($linksToDisplay), 'result_count' => count($linksToDisplay),
'search_term' => $searchTerm, 'search_term' => escape($searchTerm),
'search_tags' => $searchTags, 'search_tags' => escape($searchTags),
'search_tags_url' => array_map('urlencode', explode(' ', $searchTags)),
'visibility' => $visibility, 'visibility' => $visibility,
'links' => $linkDisp, 'links' => $linkDisp,
] ]

View file

@ -66,10 +66,18 @@ class TagCloudController extends ShaarliVisitorController
$tags = $this->formatTagsForCloud($tags); $tags = $this->formatTagsForCloud($tags);
} }
$tagsUrl = [];
foreach ($tags as $tag => $value) {
$tagsUrl[escape($tag)] = urlencode((string) $tag);
}
$searchTags = implode(' ', escape($filteringTags)); $searchTags = implode(' ', escape($filteringTags));
$searchTagsUrl = urlencode(implode(' ', $filteringTags));
$data = [ $data = [
'search_tags' => $searchTags, 'search_tags' => escape($searchTags),
'tags' => $tags, 'search_tags_url' => $searchTagsUrl,
'tags' => escape($tags),
'tags_url' => $tagsUrl,
]; ];
$this->executePageHooks('render_tag' . $type, $data, 'tag.' . $type); $this->executePageHooks('render_tag' . $type, $data, 'tag.' . $type);
$this->assignAllView($data); $this->assignAllView($data);

View file

@ -137,7 +137,7 @@ class PageBuilder
$this->tpl->assign('language', $this->conf->get('translation.language')); $this->tpl->assign('language', $this->conf->get('translation.language'));
if ($this->bookmarkService !== null) { if ($this->bookmarkService !== null) {
$this->tpl->assign('tags', $this->bookmarkService->bookmarksCountPerTag()); $this->tpl->assign('tags', escape($this->bookmarkService->bookmarksCountPerTag()));
} }
$this->tpl->assign( $this->tpl->assign(

View file

@ -555,6 +555,7 @@ function init(description) {
} }
const refreshedToken = document.getElementById('token').value; const refreshedToken = document.getElementById('token').value;
const fromtag = block.getAttribute('data-tag'); const fromtag = block.getAttribute('data-tag');
const fromtagUrl = block.getAttribute('data-tag-url');
const xhr = new XMLHttpRequest(); const xhr = new XMLHttpRequest();
xhr.open('POST', `${basePath}/admin/tags`); xhr.open('POST', `${basePath}/admin/tags`);
xhr.setRequestHeader('Content-Type', 'application/x-www-form-urlencoded'); xhr.setRequestHeader('Content-Type', 'application/x-www-form-urlencoded');
@ -564,6 +565,7 @@ function init(description) {
location.reload(); location.reload();
} else { } else {
block.setAttribute('data-tag', totag); block.setAttribute('data-tag', totag);
block.setAttribute('data-tag-url', encodeURIComponent(totag));
input.setAttribute('name', totag); input.setAttribute('name', totag);
input.setAttribute('value', totag); input.setAttribute('value', totag);
findParent(input, 'div', { class: 'rename-tag-form' }).style.display = 'none'; findParent(input, 'div', { class: 'rename-tag-form' }).style.display = 'none';
@ -571,6 +573,9 @@ function init(description) {
block block
.querySelector('a.tag-link') .querySelector('a.tag-link')
.setAttribute('href', `${basePath}/?searchtags=${encodeURIComponent(totag)}`); .setAttribute('href', `${basePath}/?searchtags=${encodeURIComponent(totag)}`);
block
.querySelector('a.count')
.setAttribute('href', `${basePath}/add-tag/${encodeURIComponent(totag)}`);
block block
.querySelector('a.rename-tag') .querySelector('a.rename-tag')
.setAttribute('href', `${basePath}/admin/tags?fromtag=${encodeURIComponent(totag)}`); .setAttribute('href', `${basePath}/admin/tags?fromtag=${encodeURIComponent(totag)}`);
@ -580,7 +585,7 @@ function init(description) {
awesomepletes = updateAwesompleteList('.rename-tag-input', existingTags, awesomepletes); awesomepletes = updateAwesompleteList('.rename-tag-input', existingTags, awesomepletes);
} }
}; };
xhr.send(`renametag=1&fromtag=${encodeURIComponent(fromtag)}&totag=${encodeURIComponent(totag)}&token=${refreshedToken}`); xhr.send(`renametag=1&fromtag=${fromtagUrl}&totag=${encodeURIComponent(totag)}&token=${refreshedToken}`);
refreshToken(basePath); refreshToken(basePath);
}); });
}); });
@ -603,6 +608,7 @@ function init(description) {
event.preventDefault(); event.preventDefault();
const block = findParent(event.target, 'div', { class: 'tag-list-item' }); const block = findParent(event.target, 'div', { class: 'tag-list-item' });
const tag = block.getAttribute('data-tag'); const tag = block.getAttribute('data-tag');
const tagUrl = block.getAttribute('data-tag-url');
const refreshedToken = document.getElementById('token').value; const refreshedToken = document.getElementById('token').value;
if (confirm(`Are you sure you want to delete the tag "${tag}"?`)) { if (confirm(`Are you sure you want to delete the tag "${tag}"?`)) {
@ -612,7 +618,7 @@ function init(description) {
xhr.onload = () => { xhr.onload = () => {
block.remove(); block.remove();
}; };
xhr.send(encodeURI(`deletetag=1&fromtag=${tag}&token=${refreshedToken}`)); xhr.send(`deletetag=1&fromtag=${tagUrl}&token=${refreshedToken}`);
refreshToken(basePath); refreshToken(basePath);
existingTags = existingTags.filter((tagItem) => tagItem !== tag); existingTags = existingTags.filter((tagItem) => tagItem !== tag);

View file

@ -94,7 +94,7 @@
{'tagged'|t} {'tagged'|t}
{loop="$exploded_tags"} {loop="$exploded_tags"}
<span class="label label-tag" title="{'Remove tag'|t}"> <span class="label label-tag" title="{'Remove tag'|t}">
<a href="{$base_path}/remove-tag/{function="urlencode($value)"}" aria-label="{'Remove tag'|t}"> <a href="{$base_path}/remove-tag/{function="$search_tags_url.$key1"}" aria-label="{'Remove tag'|t}">
{$value}<span class="remove"><i class="fa fa-times" aria-hidden="true"></i></span> {$value}<span class="remove"><i class="fa fa-times" aria-hidden="true"></i></span>
</a> </a>
</span> </span>
@ -183,7 +183,7 @@
{$tag_counter=count($value.taglist)} {$tag_counter=count($value.taglist)}
{loop="value.taglist"} {loop="value.taglist"}
<span class="label label-tag" title="{$strAddTag}"> <span class="label label-tag" title="{$strAddTag}">
<a href="{$base_path}/add-tag/{$value|urlencode}">{$value}</a> <a href="{$base_path}/add-tag/{$value1.urlencoded_taglist.$key2}">{$value}</a>
</span> </span>
{if="$tag_counter - 1 != $counter"}&middot;{/if} {if="$tag_counter - 1 != $counter"}&middot;{/if}
{/loop} {/loop}

View file

@ -15,7 +15,7 @@
<h2 class="window-title">{'Tag cloud'|t} - {$countTags} {'tags'|t}</h2> <h2 class="window-title">{'Tag cloud'|t} - {$countTags} {'tags'|t}</h2>
{if="!empty($search_tags)"} {if="!empty($search_tags)"}
<p class="center"> <p class="center">
<a href="{$base_path}/?searchtags={$search_tags|urlencode}" class="pure-button pure-button-shaarli"> <a href="{$base_path}/?searchtags={$search_tags_url}" class="pure-button pure-button-shaarli">
{'List all links with those tags'|t} {'List all links with those tags'|t}
</a> </a>
</p> </p>
@ -48,8 +48,8 @@
<div id="cloudtag" class="cloudtag-container"> <div id="cloudtag" class="cloudtag-container">
{loop="tags"} {loop="tags"}
<a href="{$base_path}/?searchtags={$key|urlencode} {$search_tags|urlencode}" style="font-size:{$value.size}em;">{$key}</a <a href="{$base_path}/?searchtags={$tags_url.$key1} {$search_tags_url}" style="font-size:{$value.size}em;">{$key}</a
><a href="{$base_path}/add-tag/{$key|urlencode}" title="{'Filter by tag'|t}" class="count">{$value.count}</a> ><a href="{$base_path}/add-tag/{$tags_url.$key1}" title="{'Filter by tag'|t}" class="count">{$value.count}</a>
{loop="$value.tag_plugin"} {loop="$value.tag_plugin"}
{$value} {$value}
{/loop} {/loop}

View file

@ -15,7 +15,7 @@
<h2 class="window-title">{'Tag list'|t} - {$countTags} {'tags'|t}</h2> <h2 class="window-title">{'Tag list'|t} - {$countTags} {'tags'|t}</h2>
{if="!empty($search_tags)"} {if="!empty($search_tags)"}
<p class="center"> <p class="center">
<a href="{$base_path}/?searchtags={$search_tags|urlencode}" class="pure-button pure-button-shaarli"> <a href="{$base_path}/?searchtags={$search_tags_url}" class="pure-button pure-button-shaarli">
{'List all links with those tags'|t} {'List all links with those tags'|t}
</a> </a>
</p> </p>
@ -47,17 +47,17 @@
<div id="taglist" class="taglist-container"> <div id="taglist" class="taglist-container">
{loop="tags"} {loop="tags"}
<div class="tag-list-item pure-g" data-tag="{$key}"> <div class="tag-list-item pure-g" data-tag="{$key}" data-tag-url="{$tags_url.$key1}">
<div class="pure-u-1"> <div class="pure-u-1">
{if="$is_logged_in===true"} {if="$is_logged_in===true"}
<a href="#" class="delete-tag" aria-label="{'Delete'|t}"><i class="fa fa-trash" aria-hidden="true"></i></a>&nbsp;&nbsp; <a href="#" class="delete-tag" aria-label="{'Delete'|t}"><i class="fa fa-trash" aria-hidden="true"></i></a>&nbsp;&nbsp;
<a href="{$base_path}/admin/tags?fromtag={$key|urlencode}" class="rename-tag" aria-label="{'Rename tag'|t}"> <a href="{$base_path}/admin/tags?fromtag={$tags_url.$key1}" class="rename-tag" aria-label="{'Rename tag'|t}">
<i class="fa fa-pencil-square-o {$key}" aria-hidden="true"></i> <i class="fa fa-pencil-square-o {$key}" aria-hidden="true"></i>
</a> </a>
{/if} {/if}
<a href="{$base_path}/add-tag/{$key|urlencode}" title="{'Filter by tag'|t}" class="count">{$value}</a> <a href="{$base_path}/add-tag/{$tags_url.$key1}" title="{'Filter by tag'|t}" class="count">{$value}</a>
<a href="{$base_path}/?searchtags={$key|urlencode} {$search_tags|urlencode}" class="tag-link">{$key}</a> <a href="{$base_path}/?searchtags={$tags_url.$key1} {$search_tags_url}" class="tag-link">{$key}</a>
{loop="$value.tag_plugin"} {loop="$value.tag_plugin"}
{$value} {$value}