From 72fbbcd6794facea2cf06d9742359d190257b00f Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Tue, 6 Oct 2020 17:30:18 +0200 Subject: 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. --- application/Utils.php | 4 ++-- application/formatter/BookmarkFormatter.php | 26 ++++++++++++++++++++++ .../controller/admin/ManageShaareController.php | 10 ++++----- .../front/controller/admin/ManageTagController.php | 4 ++-- .../controller/visitor/BookmarkListController.php | 7 +++--- .../controller/visitor/TagCloudController.php | 12 ++++++++-- application/render/PageBuilder.php | 2 +- 7 files changed, 50 insertions(+), 15 deletions(-) (limited to 'application') diff --git a/application/Utils.php b/application/Utils.php index 9c9eaaa2..bcfda65c 100644 --- a/application/Utils.php +++ b/application/Utils.php @@ -95,14 +95,14 @@ function escape($input) return null; } - if (is_bool($input)) { + if (is_bool($input) || is_int($input) || is_float($input) || $input instanceof DateTimeInterface) { return $input; } if (is_array($input)) { $out = array(); foreach ($input as $key => $value) { - $out[$key] = escape($value); + $out[escape($key)] = escape($value); } return $out; } diff --git a/application/formatter/BookmarkFormatter.php b/application/formatter/BookmarkFormatter.php index 22ba7aae..0042dafe 100644 --- a/application/formatter/BookmarkFormatter.php +++ b/application/formatter/BookmarkFormatter.php @@ -58,7 +58,9 @@ abstract class BookmarkFormatter $out['title'] = $this->formatTitle($bookmark); $out['description'] = $this->formatDescription($bookmark); $out['thumbnail'] = $this->formatThumbnail($bookmark); + $out['urlencoded_taglist'] = $this->formatUrlEncodedTagList($bookmark); $out['taglist'] = $this->formatTagList($bookmark); + $out['urlencoded_tags'] = $this->formatUrlEncodedTagString($bookmark); $out['tags'] = $this->formatTagString($bookmark); $out['sticky'] = $bookmark->isSticky(); $out['private'] = $bookmark->isPrivate(); @@ -181,6 +183,18 @@ abstract class BookmarkFormatter 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 * @@ -193,6 +207,18 @@ abstract class BookmarkFormatter 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 * Used to add specific CSS class for a link diff --git a/application/front/controller/admin/ManageShaareController.php b/application/front/controller/admin/ManageShaareController.php index 59ba2de9..bb083486 100644 --- a/application/front/controller/admin/ManageShaareController.php +++ b/application/front/controller/admin/ManageShaareController.php @@ -78,13 +78,13 @@ class ManageShaareController extends ShaarliAdminController $title = $this->container->conf->get('general.default_note_title', t('Note: ')); } - $link = escape([ + $link = [ 'title' => $title, 'url' => $url ?? '', 'description' => $description ?? '', 'tags' => $tags ?? '', 'private' => $private, - ]); + ]; } else { $formatter = $this->container->formatterFactory->getFormatter('raw'); $link = $formatter->format($bookmark); @@ -345,14 +345,14 @@ class ManageShaareController extends ShaarliAdminController $tags[BookmarkMarkdownFormatter::NO_MD_TAG] = 1; } - $data = [ + $data = escape([ 'link' => $link, 'link_is_new' => $isNew, - 'http_referer' => escape($this->container->environment['HTTP_REFERER'] ?? ''), + 'http_referer' => $this->container->environment['HTTP_REFERER'] ?? '', 'source' => $request->getParam('source') ?? '', 'tags' => $tags, 'default_private_links' => $this->container->conf->get('privacy.default_private_links', false), - ]; + ]); $this->executePageHooks('render_editlink', $data, TemplatePage::EDIT_LINK); diff --git a/application/front/controller/admin/ManageTagController.php b/application/front/controller/admin/ManageTagController.php index 0380ef1f..2065c3e2 100644 --- a/application/front/controller/admin/ManageTagController.php +++ b/application/front/controller/admin/ManageTagController.php @@ -41,8 +41,8 @@ class ManageTagController extends ShaarliAdminController $isDelete = null !== $request->getParam('deletetag') && null === $request->getParam('renametag'); - $fromTag = escape(trim($request->getParam('fromtag') ?? '')); - $toTag = escape(trim($request->getParam('totag') ?? '')); + $fromTag = trim($request->getParam('fromtag') ?? ''); + $toTag = trim($request->getParam('totag') ?? ''); if (0 === strlen($fromTag) || false === $isDelete && 0 === strlen($toTag)) { $this->saveWarningMessage(t('Invalid tags provided.')); diff --git a/application/front/controller/visitor/BookmarkListController.php b/application/front/controller/visitor/BookmarkListController.php index 2988bee6..18368751 100644 --- a/application/front/controller/visitor/BookmarkListController.php +++ b/application/front/controller/visitor/BookmarkListController.php @@ -34,7 +34,7 @@ class BookmarkListController extends ShaarliVisitorController $formatter = $this->container->formatterFactory->getFormatter(); $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') ?? ''));; // Filter bookmarks according search parameters. @@ -104,8 +104,9 @@ class BookmarkListController extends ShaarliVisitorController 'page_current' => $page, 'page_max' => $pageCount, 'result_count' => count($linksToDisplay), - 'search_term' => $searchTerm, - 'search_tags' => $searchTags, + 'search_term' => escape($searchTerm), + 'search_tags' => escape($searchTags), + 'search_tags_url' => array_map('urlencode', explode(' ', $searchTags)), 'visibility' => $visibility, 'links' => $linkDisp, ] diff --git a/application/front/controller/visitor/TagCloudController.php b/application/front/controller/visitor/TagCloudController.php index f9c529bc..76ed7690 100644 --- a/application/front/controller/visitor/TagCloudController.php +++ b/application/front/controller/visitor/TagCloudController.php @@ -66,10 +66,18 @@ class TagCloudController extends ShaarliVisitorController $tags = $this->formatTagsForCloud($tags); } + $tagsUrl = []; + foreach ($tags as $tag => $value) { + $tagsUrl[escape($tag)] = urlencode((string) $tag); + } + $searchTags = implode(' ', escape($filteringTags)); + $searchTagsUrl = urlencode(implode(' ', $filteringTags)); $data = [ - 'search_tags' => $searchTags, - 'tags' => $tags, + 'search_tags' => escape($searchTags), + 'search_tags_url' => $searchTagsUrl, + 'tags' => escape($tags), + 'tags_url' => $tagsUrl, ]; $this->executePageHooks('render_tag' . $type, $data, 'tag.' . $type); $this->assignAllView($data); diff --git a/application/render/PageBuilder.php b/application/render/PageBuilder.php index c52e3b76..41b357dd 100644 --- a/application/render/PageBuilder.php +++ b/application/render/PageBuilder.php @@ -137,7 +137,7 @@ class PageBuilder $this->tpl->assign('language', $this->conf->get('translation.language')); if ($this->bookmarkService !== null) { - $this->tpl->assign('tags', $this->bookmarkService->bookmarksCountPerTag()); + $this->tpl->assign('tags', escape($this->bookmarkService->bookmarksCountPerTag())); } $this->tpl->assign( -- cgit v1.2.3