aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorArthurHoaro <arthur@hoa.ro>2020-10-08 08:19:06 +0200
committerGitHub <noreply@github.com>2020-10-08 08:19:06 +0200
commit7f1bb5553b7427bd3a9e5b119f99c2ac3509c607 (patch)
treea4d6f446ec861f9a7591edb31f322e2a846b2bac
parentdf25b28dcd3cde54d42c18a55a810daa82bf5727 (diff)
parent72fbbcd6794facea2cf06d9742359d190257b00f (diff)
downloadShaarli-7f1bb5553b7427bd3a9e5b119f99c2ac3509c607.tar.gz
Shaarli-7f1bb5553b7427bd3a9e5b119f99c2ac3509c607.tar.zst
Shaarli-7f1bb5553b7427bd3a9e5b119f99c2ac3509c607.zip
Merge pull request #1585 from ArthurHoaro/fix/xss-and-tag-search
Security: fix multiple XSS vulnerabilities + fix search tags with special chars
-rw-r--r--application/Utils.php4
-rw-r--r--application/formatter/BookmarkFormatter.php26
-rw-r--r--application/front/controller/admin/ManageShaareController.php10
-rw-r--r--application/front/controller/admin/ManageTagController.php4
-rw-r--r--application/front/controller/visitor/BookmarkListController.php7
-rw-r--r--application/front/controller/visitor/TagCloudController.php12
-rw-r--r--application/render/PageBuilder.php2
-rw-r--r--assets/default/js/base.js10
-rw-r--r--tpl/default/linklist.html4
-rw-r--r--tpl/default/tag.cloud.html6
-rw-r--r--tpl/default/tag.list.html10
11 files changed, 68 insertions, 27 deletions
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)
95 return null; 95 return null;
96 } 96 }
97 97
98 if (is_bool($input)) { 98 if (is_bool($input) || is_int($input) || is_float($input) || $input instanceof DateTimeInterface) {
99 return $input; 99 return $input;
100 } 100 }
101 101
102 if (is_array($input)) { 102 if (is_array($input)) {
103 $out = array(); 103 $out = array();
104 foreach ($input as $key => $value) { 104 foreach ($input as $key => $value) {
105 $out[$key] = escape($value); 105 $out[escape($key)] = escape($value);
106 } 106 }
107 return $out; 107 return $out;
108 } 108 }
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
58 $out['title'] = $this->formatTitle($bookmark); 58 $out['title'] = $this->formatTitle($bookmark);
59 $out['description'] = $this->formatDescription($bookmark); 59 $out['description'] = $this->formatDescription($bookmark);
60 $out['thumbnail'] = $this->formatThumbnail($bookmark); 60 $out['thumbnail'] = $this->formatThumbnail($bookmark);
61 $out['urlencoded_taglist'] = $this->formatUrlEncodedTagList($bookmark);
61 $out['taglist'] = $this->formatTagList($bookmark); 62 $out['taglist'] = $this->formatTagList($bookmark);
63 $out['urlencoded_tags'] = $this->formatUrlEncodedTagString($bookmark);
62 $out['tags'] = $this->formatTagString($bookmark); 64 $out['tags'] = $this->formatTagString($bookmark);
63 $out['sticky'] = $bookmark->isSticky(); 65 $out['sticky'] = $bookmark->isSticky();
64 $out['private'] = $bookmark->isPrivate(); 66 $out['private'] = $bookmark->isPrivate();
@@ -182,6 +184,18 @@ abstract class BookmarkFormatter
182 } 184 }
183 185
184 /** 186 /**
187 * Format Url Encoded Tags
188 *
189 * @param Bookmark $bookmark instance
190 *
191 * @return array formatted Tags
192 */
193 protected function formatUrlEncodedTagList($bookmark)
194 {
195 return array_map('urlencode', $this->filterTagList($bookmark->getTags()));
196 }
197
198 /**
185 * Format TagString 199 * Format TagString
186 * 200 *
187 * @param Bookmark $bookmark instance 201 * @param Bookmark $bookmark instance
@@ -194,6 +208,18 @@ abstract class BookmarkFormatter
194 } 208 }
195 209
196 /** 210 /**
211 * Format TagString
212 *
213 * @param Bookmark $bookmark instance
214 *
215 * @return string formatted TagString
216 */
217 protected function formatUrlEncodedTagString($bookmark)
218 {
219 return implode(' ', $this->formatUrlEncodedTagList($bookmark));
220 }
221
222 /**
197 * Format Class 223 * Format Class
198 * Used to add specific CSS class for a link 224 * Used to add specific CSS class for a link
199 * 225 *
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
78 $title = $this->container->conf->get('general.default_note_title', t('Note: ')); 78 $title = $this->container->conf->get('general.default_note_title', t('Note: '));
79 } 79 }
80 80
81 $link = escape([ 81 $link = [
82 'title' => $title, 82 'title' => $title,
83 'url' => $url ?? '', 83 'url' => $url ?? '',
84 'description' => $description ?? '', 84 'description' => $description ?? '',
85 'tags' => $tags ?? '', 85 'tags' => $tags ?? '',
86 'private' => $private, 86 'private' => $private,
87 ]); 87 ];
88 } else { 88 } else {
89 $formatter = $this->container->formatterFactory->getFormatter('raw'); 89 $formatter = $this->container->formatterFactory->getFormatter('raw');
90 $link = $formatter->format($bookmark); 90 $link = $formatter->format($bookmark);
@@ -345,14 +345,14 @@ class ManageShaareController extends ShaarliAdminController
345 $tags[BookmarkMarkdownFormatter::NO_MD_TAG] = 1; 345 $tags[BookmarkMarkdownFormatter::NO_MD_TAG] = 1;
346 } 346 }
347 347
348 $data = [ 348 $data = escape([
349 'link' => $link, 349 'link' => $link,
350 'link_is_new' => $isNew, 350 'link_is_new' => $isNew,
351 'http_referer' => escape($this->container->environment['HTTP_REFERER'] ?? ''), 351 'http_referer' => $this->container->environment['HTTP_REFERER'] ?? '',
352 'source' => $request->getParam('source') ?? '', 352 'source' => $request->getParam('source') ?? '',
353 'tags' => $tags, 353 'tags' => $tags,
354 'default_private_links' => $this->container->conf->get('privacy.default_private_links', false), 354 'default_private_links' => $this->container->conf->get('privacy.default_private_links', false),
355 ]; 355 ]);
356 356
357 $this->executePageHooks('render_editlink', $data, TemplatePage::EDIT_LINK); 357 $this->executePageHooks('render_editlink', $data, TemplatePage::EDIT_LINK);
358 358
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
41 41
42 $isDelete = null !== $request->getParam('deletetag') && null === $request->getParam('renametag'); 42 $isDelete = null !== $request->getParam('deletetag') && null === $request->getParam('renametag');
43 43
44 $fromTag = escape(trim($request->getParam('fromtag') ?? '')); 44 $fromTag = trim($request->getParam('fromtag') ?? '');
45 $toTag = escape(trim($request->getParam('totag') ?? '')); 45 $toTag = trim($request->getParam('totag') ?? '');
46 46
47 if (0 === strlen($fromTag) || false === $isDelete && 0 === strlen($toTag)) { 47 if (0 === strlen($fromTag) || false === $isDelete && 0 === strlen($toTag)) {
48 $this->saveWarningMessage(t('Invalid tags provided.')); 48 $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
34 $formatter = $this->container->formatterFactory->getFormatter(); 34 $formatter = $this->container->formatterFactory->getFormatter();
35 $formatter->addContextData('base_path', $this->container->basePath); 35 $formatter->addContextData('base_path', $this->container->basePath);
36 36
37 $searchTags = escape(normalize_spaces($request->getParam('searchtags') ?? '')); 37 $searchTags = normalize_spaces($request->getParam('searchtags') ?? '');
38 $searchTerm = escape(normalize_spaces($request->getParam('searchterm') ?? ''));; 38 $searchTerm = escape(normalize_spaces($request->getParam('searchterm') ?? ''));;
39 39
40 // Filter bookmarks according search parameters. 40 // Filter bookmarks according search parameters.
@@ -104,8 +104,9 @@ class BookmarkListController extends ShaarliVisitorController
104 'page_current' => $page, 104 'page_current' => $page,
105 'page_max' => $pageCount, 105 'page_max' => $pageCount,
106 'result_count' => count($linksToDisplay), 106 'result_count' => count($linksToDisplay),
107 'search_term' => $searchTerm, 107 'search_term' => escape($searchTerm),
108 'search_tags' => $searchTags, 108 'search_tags' => escape($searchTags),
109 'search_tags_url' => array_map('urlencode', explode(' ', $searchTags)),
109 'visibility' => $visibility, 110 'visibility' => $visibility,
110 'links' => $linkDisp, 111 'links' => $linkDisp,
111 ] 112 ]
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
66 $tags = $this->formatTagsForCloud($tags); 66 $tags = $this->formatTagsForCloud($tags);
67 } 67 }
68 68
69 $tagsUrl = [];
70 foreach ($tags as $tag => $value) {
71 $tagsUrl[escape($tag)] = urlencode((string) $tag);
72 }
73
69 $searchTags = implode(' ', escape($filteringTags)); 74 $searchTags = implode(' ', escape($filteringTags));
75 $searchTagsUrl = urlencode(implode(' ', $filteringTags));
70 $data = [ 76 $data = [
71 'search_tags' => $searchTags, 77 'search_tags' => escape($searchTags),
72 'tags' => $tags, 78 'search_tags_url' => $searchTagsUrl,
79 'tags' => escape($tags),
80 'tags_url' => $tagsUrl,
73 ]; 81 ];
74 $this->executePageHooks('render_tag' . $type, $data, 'tag.' . $type); 82 $this->executePageHooks('render_tag' . $type, $data, 'tag.' . $type);
75 $this->assignAllView($data); 83 $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
137 $this->tpl->assign('language', $this->conf->get('translation.language')); 137 $this->tpl->assign('language', $this->conf->get('translation.language'));
138 138
139 if ($this->bookmarkService !== null) { 139 if ($this->bookmarkService !== null) {
140 $this->tpl->assign('tags', $this->bookmarkService->bookmarksCountPerTag()); 140 $this->tpl->assign('tags', escape($this->bookmarkService->bookmarksCountPerTag()));
141 } 141 }
142 142
143 $this->tpl->assign( 143 $this->tpl->assign(
diff --git a/assets/default/js/base.js b/assets/default/js/base.js
index d9933152..be986ae0 100644
--- a/assets/default/js/base.js
+++ b/assets/default/js/base.js
@@ -555,6 +555,7 @@ function init(description) {
555 } 555 }
556 const refreshedToken = document.getElementById('token').value; 556 const refreshedToken = document.getElementById('token').value;
557 const fromtag = block.getAttribute('data-tag'); 557 const fromtag = block.getAttribute('data-tag');
558 const fromtagUrl = block.getAttribute('data-tag-url');
558 const xhr = new XMLHttpRequest(); 559 const xhr = new XMLHttpRequest();
559 xhr.open('POST', `${basePath}/admin/tags`); 560 xhr.open('POST', `${basePath}/admin/tags`);
560 xhr.setRequestHeader('Content-Type', 'application/x-www-form-urlencoded'); 561 xhr.setRequestHeader('Content-Type', 'application/x-www-form-urlencoded');
@@ -564,6 +565,7 @@ function init(description) {
564 location.reload(); 565 location.reload();
565 } else { 566 } else {
566 block.setAttribute('data-tag', totag); 567 block.setAttribute('data-tag', totag);
568 block.setAttribute('data-tag-url', encodeURIComponent(totag));
567 input.setAttribute('name', totag); 569 input.setAttribute('name', totag);
568 input.setAttribute('value', totag); 570 input.setAttribute('value', totag);
569 findParent(input, 'div', { class: 'rename-tag-form' }).style.display = 'none'; 571 findParent(input, 'div', { class: 'rename-tag-form' }).style.display = 'none';
@@ -572,6 +574,9 @@ function init(description) {
572 .querySelector('a.tag-link') 574 .querySelector('a.tag-link')
573 .setAttribute('href', `${basePath}/?searchtags=${encodeURIComponent(totag)}`); 575 .setAttribute('href', `${basePath}/?searchtags=${encodeURIComponent(totag)}`);
574 block 576 block
577 .querySelector('a.count')
578 .setAttribute('href', `${basePath}/add-tag/${encodeURIComponent(totag)}`);
579 block
575 .querySelector('a.rename-tag') 580 .querySelector('a.rename-tag')
576 .setAttribute('href', `${basePath}/admin/tags?fromtag=${encodeURIComponent(totag)}`); 581 .setAttribute('href', `${basePath}/admin/tags?fromtag=${encodeURIComponent(totag)}`);
577 582
@@ -580,7 +585,7 @@ function init(description) {
580 awesomepletes = updateAwesompleteList('.rename-tag-input', existingTags, awesomepletes); 585 awesomepletes = updateAwesompleteList('.rename-tag-input', existingTags, awesomepletes);
581 } 586 }
582 }; 587 };
583 xhr.send(`renametag=1&fromtag=${encodeURIComponent(fromtag)}&totag=${encodeURIComponent(totag)}&token=${refreshedToken}`); 588 xhr.send(`renametag=1&fromtag=${fromtagUrl}&totag=${encodeURIComponent(totag)}&token=${refreshedToken}`);
584 refreshToken(basePath); 589 refreshToken(basePath);
585 }); 590 });
586 }); 591 });
@@ -603,6 +608,7 @@ function init(description) {
603 event.preventDefault(); 608 event.preventDefault();
604 const block = findParent(event.target, 'div', { class: 'tag-list-item' }); 609 const block = findParent(event.target, 'div', { class: 'tag-list-item' });
605 const tag = block.getAttribute('data-tag'); 610 const tag = block.getAttribute('data-tag');
611 const tagUrl = block.getAttribute('data-tag-url');
606 const refreshedToken = document.getElementById('token').value; 612 const refreshedToken = document.getElementById('token').value;
607 613
608 if (confirm(`Are you sure you want to delete the tag "${tag}"?`)) { 614 if (confirm(`Are you sure you want to delete the tag "${tag}"?`)) {
@@ -612,7 +618,7 @@ function init(description) {
612 xhr.onload = () => { 618 xhr.onload = () => {
613 block.remove(); 619 block.remove();
614 }; 620 };
615 xhr.send(encodeURI(`deletetag=1&fromtag=${tag}&token=${refreshedToken}`)); 621 xhr.send(`deletetag=1&fromtag=${tagUrl}&token=${refreshedToken}`);
616 refreshToken(basePath); 622 refreshToken(basePath);
617 623
618 existingTags = existingTags.filter((tagItem) => tagItem !== tag); 624 existingTags = existingTags.filter((tagItem) => tagItem !== tag);
diff --git a/tpl/default/linklist.html b/tpl/default/linklist.html
index 2475f5fd..b08773d8 100644
--- a/tpl/default/linklist.html
+++ b/tpl/default/linklist.html
@@ -94,7 +94,7 @@
94 {'tagged'|t} 94 {'tagged'|t}
95 {loop="$exploded_tags"} 95 {loop="$exploded_tags"}
96 <span class="label label-tag" title="{'Remove tag'|t}"> 96 <span class="label label-tag" title="{'Remove tag'|t}">
97 <a href="{$base_path}/remove-tag/{function="urlencode($value)"}" aria-label="{'Remove tag'|t}"> 97 <a href="{$base_path}/remove-tag/{function="$search_tags_url.$key1"}" aria-label="{'Remove tag'|t}">
98 {$value}<span class="remove"><i class="fa fa-times" aria-hidden="true"></i></span> 98 {$value}<span class="remove"><i class="fa fa-times" aria-hidden="true"></i></span>
99 </a> 99 </a>
100 </span> 100 </span>
@@ -183,7 +183,7 @@
183 {$tag_counter=count($value.taglist)} 183 {$tag_counter=count($value.taglist)}
184 {loop="value.taglist"} 184 {loop="value.taglist"}
185 <span class="label label-tag" title="{$strAddTag}"> 185 <span class="label label-tag" title="{$strAddTag}">
186 <a href="{$base_path}/add-tag/{$value|urlencode}">{$value}</a> 186 <a href="{$base_path}/add-tag/{$value1.urlencoded_taglist.$key2}">{$value}</a>
187 </span> 187 </span>
188 {if="$tag_counter - 1 != $counter"}&middot;{/if} 188 {if="$tag_counter - 1 != $counter"}&middot;{/if}
189 {/loop} 189 {/loop}
diff --git a/tpl/default/tag.cloud.html b/tpl/default/tag.cloud.html
index 024882ec..c067e1d4 100644
--- a/tpl/default/tag.cloud.html
+++ b/tpl/default/tag.cloud.html
@@ -15,7 +15,7 @@
15 <h2 class="window-title">{'Tag cloud'|t} - {$countTags} {'tags'|t}</h2> 15 <h2 class="window-title">{'Tag cloud'|t} - {$countTags} {'tags'|t}</h2>
16 {if="!empty($search_tags)"} 16 {if="!empty($search_tags)"}
17 <p class="center"> 17 <p class="center">
18 <a href="{$base_path}/?searchtags={$search_tags|urlencode}" class="pure-button pure-button-shaarli"> 18 <a href="{$base_path}/?searchtags={$search_tags_url}" class="pure-button pure-button-shaarli">
19 {'List all links with those tags'|t} 19 {'List all links with those tags'|t}
20 </a> 20 </a>
21 </p> 21 </p>
@@ -48,8 +48,8 @@
48 48
49 <div id="cloudtag" class="cloudtag-container"> 49 <div id="cloudtag" class="cloudtag-container">
50 {loop="tags"} 50 {loop="tags"}
51 <a href="{$base_path}/?searchtags={$key|urlencode} {$search_tags|urlencode}" style="font-size:{$value.size}em;">{$key}</a 51 <a href="{$base_path}/?searchtags={$tags_url.$key1} {$search_tags_url}" style="font-size:{$value.size}em;">{$key}</a
52 ><a href="{$base_path}/add-tag/{$key|urlencode}" title="{'Filter by tag'|t}" class="count">{$value.count}</a> 52 ><a href="{$base_path}/add-tag/{$tags_url.$key1}" title="{'Filter by tag'|t}" class="count">{$value.count}</a>
53 {loop="$value.tag_plugin"} 53 {loop="$value.tag_plugin"}
54 {$value} 54 {$value}
55 {/loop} 55 {/loop}
diff --git a/tpl/default/tag.list.html b/tpl/default/tag.list.html
index 99ae44d2..96e7fbe0 100644
--- a/tpl/default/tag.list.html
+++ b/tpl/default/tag.list.html
@@ -15,7 +15,7 @@
15 <h2 class="window-title">{'Tag list'|t} - {$countTags} {'tags'|t}</h2> 15 <h2 class="window-title">{'Tag list'|t} - {$countTags} {'tags'|t}</h2>
16 {if="!empty($search_tags)"} 16 {if="!empty($search_tags)"}
17 <p class="center"> 17 <p class="center">
18 <a href="{$base_path}/?searchtags={$search_tags|urlencode}" class="pure-button pure-button-shaarli"> 18 <a href="{$base_path}/?searchtags={$search_tags_url}" class="pure-button pure-button-shaarli">
19 {'List all links with those tags'|t} 19 {'List all links with those tags'|t}
20 </a> 20 </a>
21 </p> 21 </p>
@@ -47,17 +47,17 @@
47 47
48 <div id="taglist" class="taglist-container"> 48 <div id="taglist" class="taglist-container">
49 {loop="tags"} 49 {loop="tags"}
50 <div class="tag-list-item pure-g" data-tag="{$key}"> 50 <div class="tag-list-item pure-g" data-tag="{$key}" data-tag-url="{$tags_url.$key1}">
51 <div class="pure-u-1"> 51 <div class="pure-u-1">
52 {if="$is_logged_in===true"} 52 {if="$is_logged_in===true"}
53 <a href="#" class="delete-tag" aria-label="{'Delete'|t}"><i class="fa fa-trash" aria-hidden="true"></i></a>&nbsp;&nbsp; 53 <a href="#" class="delete-tag" aria-label="{'Delete'|t}"><i class="fa fa-trash" aria-hidden="true"></i></a>&nbsp;&nbsp;
54 <a href="{$base_path}/admin/tags?fromtag={$key|urlencode}" class="rename-tag" aria-label="{'Rename tag'|t}"> 54 <a href="{$base_path}/admin/tags?fromtag={$tags_url.$key1}" class="rename-tag" aria-label="{'Rename tag'|t}">
55 <i class="fa fa-pencil-square-o {$key}" aria-hidden="true"></i> 55 <i class="fa fa-pencil-square-o {$key}" aria-hidden="true"></i>
56 </a> 56 </a>
57 {/if} 57 {/if}
58 58
59 <a href="{$base_path}/add-tag/{$key|urlencode}" title="{'Filter by tag'|t}" class="count">{$value}</a> 59 <a href="{$base_path}/add-tag/{$tags_url.$key1}" title="{'Filter by tag'|t}" class="count">{$value}</a>
60 <a href="{$base_path}/?searchtags={$key|urlencode} {$search_tags|urlencode}" class="tag-link">{$key}</a> 60 <a href="{$base_path}/?searchtags={$tags_url.$key1} {$search_tags_url}" class="tag-link">{$key}</a>
61 61
62 {loop="$value.tag_plugin"} 62 {loop="$value.tag_plugin"}
63 {$value} 63 {$value}