aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorArthurHoaro <arthur@hoa.ro>2017-05-31 17:52:19 +0200
committerGitHub <noreply@github.com>2017-05-31 17:52:19 +0200
commitac94db1e36c77d52c316b5fa4e8e36b9d1e38b9e (patch)
treeb235a4ed0e5291d7ad2f008df5bbed4d43200cbe
parent268309df5d8110f516940be06e9481d66f3fb5d6 (diff)
parent86ceea054f5f85157b04473bac5bfb6ff86ca31f (diff)
downloadShaarli-ac94db1e36c77d52c316b5fa4e8e36b9d1e38b9e.tar.gz
Shaarli-ac94db1e36c77d52c316b5fa4e8e36b9d1e38b9e.tar.zst
Shaarli-ac94db1e36c77d52c316b5fa4e8e36b9d1e38b9e.zip
Merge pull request #880 from ArthurHoaro/hotfix/allowed-protocols
Add a whitelist of protocols for URLs
-rw-r--r--application/Url.php24
-rw-r--r--application/config/ConfigManager.php1
-rw-r--r--index.php8
-rw-r--r--plugins/markdown/markdown.php37
-rw-r--r--tests/Url/WhitelistProtocolsTest.php63
-rw-r--r--tests/plugins/PluginMarkdownTest.php11
-rw-r--r--tests/plugins/resources/markdown.html11
-rw-r--r--tests/plugins/resources/markdown.md12
8 files changed, 151 insertions, 16 deletions
diff --git a/application/Url.php b/application/Url.php
index 25a62a8a..b3759377 100644
--- a/application/Url.php
+++ b/application/Url.php
@@ -64,6 +64,30 @@ function add_trailing_slash($url)
64} 64}
65 65
66/** 66/**
67 * Replace not whitelisted protocols by 'http://' from given URL.
68 *
69 * @param string $url URL to clean
70 * @param array $protocols List of allowed protocols (aside from http(s)).
71 *
72 * @return string URL with allowed protocol
73 */
74function whitelist_protocols($url, $protocols)
75{
76 if (startsWith($url, '?') || startsWith($url, '/')) {
77 return $url;
78 }
79 $protocols = array_merge(['http', 'https'], $protocols);
80 $protocol = preg_match('#^(\w+):/?/?#', $url, $match);
81 // Protocol not allowed: we remove it and replace it with http
82 if ($protocol === 1 && ! in_array($match[1], $protocols)) {
83 $url = str_replace($match[0], 'http://', $url);
84 } else if ($protocol !== 1) {
85 $url = 'http://' . $url;
86 }
87 return $url;
88}
89
90/**
67 * URL representation and cleanup utilities 91 * URL representation and cleanup utilities
68 * 92 *
69 * Form 93 * Form
diff --git a/application/config/ConfigManager.php b/application/config/ConfigManager.php
index 86a917fb..8eab26f1 100644
--- a/application/config/ConfigManager.php
+++ b/application/config/ConfigManager.php
@@ -312,6 +312,7 @@ class ConfigManager
312 $this->setEmpty('security.ban_duration', 1800); 312 $this->setEmpty('security.ban_duration', 1800);
313 $this->setEmpty('security.session_protection_disabled', false); 313 $this->setEmpty('security.session_protection_disabled', false);
314 $this->setEmpty('security.open_shaarli', false); 314 $this->setEmpty('security.open_shaarli', false);
315 $this->setEmpty('security.allowed_protocols', ['ftp', 'ftps', 'magnet']);
315 316
316 $this->setEmpty('general.header_link', '?'); 317 $this->setEmpty('general.header_link', '?');
317 $this->setEmpty('general.links_per_page', 20); 318 $this->setEmpty('general.links_per_page', 20);
diff --git a/index.php b/index.php
index 92eb443b..823eb8de 100644
--- a/index.php
+++ b/index.php
@@ -1256,13 +1256,7 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history)
1256 // Remove duplicates. 1256 // Remove duplicates.
1257 $tags = implode(' ', array_unique(explode(' ', $tags))); 1257 $tags = implode(' ', array_unique(explode(' ', $tags)));
1258 1258
1259 $url = trim($_POST['lf_url']); 1259 $url = whitelist_protocols(trim($_POST['lf_url']), $conf->get('security.allowed_protocols'));
1260 if (! startsWith($url, 'http:') && ! startsWith($url, 'https:')
1261 && ! startsWith($url, 'ftp:') && ! startsWith($url, 'magnet:')
1262 && ! startsWith($url, '?') && ! startsWith($url, 'javascript:')
1263 ) {
1264 $url = 'http://' . $url;
1265 }
1266 1260
1267 $link = array( 1261 $link = array(
1268 'id' => $id, 1262 'id' => $id,
diff --git a/plugins/markdown/markdown.php b/plugins/markdown/markdown.php
index de7c823d..772c56e8 100644
--- a/plugins/markdown/markdown.php
+++ b/plugins/markdown/markdown.php
@@ -26,7 +26,11 @@ function hook_markdown_render_linklist($data, $conf)
26 $value = stripNoMarkdownTag($value); 26 $value = stripNoMarkdownTag($value);
27 continue; 27 continue;
28 } 28 }
29 $value['description'] = process_markdown($value['description'], $conf->get('security.markdown_escape', true)); 29 $value['description'] = process_markdown(
30 $value['description'],
31 $conf->get('security.markdown_escape', true),
32 $conf->get('security.allowed_protocols')
33 );
30 } 34 }
31 return $data; 35 return $data;
32} 36}
@@ -46,7 +50,11 @@ function hook_markdown_render_feed($data, $conf)
46 $value = stripNoMarkdownTag($value); 50 $value = stripNoMarkdownTag($value);
47 continue; 51 continue;
48 } 52 }
49 $value['description'] = process_markdown($value['description'], $conf->get('security.markdown_escape', true)); 53 $value['description'] = process_markdown(
54 $value['description'],
55 $conf->get('security.markdown_escape', true),
56 $conf->get('security.allowed_protocols')
57 );
50 } 58 }
51 59
52 return $data; 60 return $data;
@@ -71,7 +79,8 @@ function hook_markdown_render_daily($data, $conf)
71 } 79 }
72 $value2['formatedDescription'] = process_markdown( 80 $value2['formatedDescription'] = process_markdown(
73 $value2['formatedDescription'], 81 $value2['formatedDescription'],
74 $conf->get('security.markdown_escape', true) 82 $conf->get('security.markdown_escape', true),
83 $conf->get('security.allowed_protocols')
75 ); 84 );
76 } 85 }
77 } 86 }
@@ -232,6 +241,25 @@ function reverse_space2nbsp($description)
232} 241}
233 242
234/** 243/**
244 * Replace not whitelisted protocols with http:// in given description.
245 *
246 * @param string $description input description text.
247 * @param array $allowedProtocols list of allowed protocols.
248 *
249 * @return string $description without malicious link.
250 */
251function filter_protocols($description, $allowedProtocols)
252{
253 return preg_replace_callback(
254 '#]\((.*?)\)#is',
255 function ($match) use ($allowedProtocols) {
256 return ']('. whitelist_protocols($match[1], $allowedProtocols) .')';
257 },
258 $description
259 );
260}
261
262/**
235 * Remove dangerous HTML tags (tags, iframe, etc.). 263 * Remove dangerous HTML tags (tags, iframe, etc.).
236 * Doesn't affect <code> content (already escaped by Parsedown). 264 * Doesn't affect <code> content (already escaped by Parsedown).
237 * 265 *
@@ -275,7 +303,7 @@ function sanitize_html($description)
275 * 303 *
276 * @return string HTML processed $description. 304 * @return string HTML processed $description.
277 */ 305 */
278function process_markdown($description, $escape = true) 306function process_markdown($description, $escape = true, $allowedProtocols = [])
279{ 307{
280 $parsedown = new Parsedown(); 308 $parsedown = new Parsedown();
281 309
@@ -283,6 +311,7 @@ function process_markdown($description, $escape = true)
283 $processedDescription = reverse_nl2br($processedDescription); 311 $processedDescription = reverse_nl2br($processedDescription);
284 $processedDescription = reverse_space2nbsp($processedDescription); 312 $processedDescription = reverse_space2nbsp($processedDescription);
285 $processedDescription = reverse_text2clickable($processedDescription); 313 $processedDescription = reverse_text2clickable($processedDescription);
314 $processedDescription = filter_protocols($processedDescription, $allowedProtocols);
286 $processedDescription = unescape($processedDescription); 315 $processedDescription = unescape($processedDescription);
287 $processedDescription = $parsedown 316 $processedDescription = $parsedown
288 ->setMarkupEscaped($escape) 317 ->setMarkupEscaped($escape)
diff --git a/tests/Url/WhitelistProtocolsTest.php b/tests/Url/WhitelistProtocolsTest.php
new file mode 100644
index 00000000..a3156804
--- /dev/null
+++ b/tests/Url/WhitelistProtocolsTest.php
@@ -0,0 +1,63 @@
1<?php
2
3require_once 'application/Url.php';
4
5use Shaarli\Config\ConfigManager;
6
7/**
8 * Class WhitelistProtocolsTest
9 *
10 * Test whitelist_protocols() function of Url.
11 */
12class WhitelistProtocolsTest extends PHPUnit_Framework_TestCase
13{
14 /**
15 * Test whitelist_protocols() on a note (relative URL).
16 */
17 public function testWhitelistProtocolsRelative()
18 {
19 $whitelist = ['ftp', 'magnet'];
20 $url = '?12443564';
21 $this->assertEquals($url, whitelist_protocols($url, $whitelist));
22 $url = '/path.jpg';
23 $this->assertEquals($url, whitelist_protocols($url, $whitelist));
24 }
25
26 /**
27 * Test whitelist_protocols() on a note (relative URL).
28 */
29 public function testWhitelistProtocolMissing()
30 {
31 $whitelist = ['ftp', 'magnet'];
32 $url = 'test.tld/path/?query=value#hash';
33 $this->assertEquals('http://'. $url, whitelist_protocols($url, $whitelist));
34 }
35
36 /**
37 * Test whitelist_protocols() with allowed protocols.
38 */
39 public function testWhitelistAllowedProtocol()
40 {
41 $whitelist = ['ftp', 'magnet'];
42 $url = 'http://test.tld/path/?query=value#hash';
43 $this->assertEquals($url, whitelist_protocols($url, $whitelist));
44 $url = 'https://test.tld/path/?query=value#hash';
45 $this->assertEquals($url, whitelist_protocols($url, $whitelist));
46 $url = 'ftp://test.tld/path/?query=value#hash';
47 $this->assertEquals($url, whitelist_protocols($url, $whitelist));
48 $url = 'magnet:test.tld/path/?query=value#hash';
49 $this->assertEquals($url, whitelist_protocols($url, $whitelist));
50 }
51
52 /**
53 * Test whitelist_protocols() with allowed protocols.
54 */
55 public function testWhitelistDisallowedProtocol()
56 {
57 $whitelist = ['ftp', 'magnet'];
58 $url = 'javascript:alert("xss");';
59 $this->assertEquals('http://alert("xss");', whitelist_protocols($url, $whitelist));
60 $url = 'other://test.tld/path/?query=value#hash';
61 $this->assertEquals('http://test.tld/path/?query=value#hash', whitelist_protocols($url, $whitelist));
62 }
63}
diff --git a/tests/plugins/PluginMarkdownTest.php b/tests/plugins/PluginMarkdownTest.php
index d8180ad6..96891f1f 100644
--- a/tests/plugins/PluginMarkdownTest.php
+++ b/tests/plugins/PluginMarkdownTest.php
@@ -26,6 +26,7 @@ class PluginMarkdownTest extends PHPUnit_Framework_TestCase
26 { 26 {
27 PluginManager::$PLUGINS_PATH = 'plugins'; 27 PluginManager::$PLUGINS_PATH = 'plugins';
28 $this->conf = new ConfigManager('tests/utils/config/configJson'); 28 $this->conf = new ConfigManager('tests/utils/config/configJson');
29 $this->conf->set('security.allowed_protocols', ['ftp', 'magnet']);
29 } 30 }
30 31
31 /** 32 /**
@@ -183,15 +184,19 @@ class PluginMarkdownTest extends PHPUnit_Framework_TestCase
183 } 184 }
184 185
185 /** 186 /**
186 * Test hashtag links processed with markdown. 187 * Make sure that the generated HTML match the reference HTML file.
187 */ 188 */
188 public function testMarkdownHashtagLinks() 189 public function testMarkdownGlobalProcessDescription()
189 { 190 {
190 $md = file_get_contents('tests/plugins/resources/markdown.md'); 191 $md = file_get_contents('tests/plugins/resources/markdown.md');
191 $md = format_description($md); 192 $md = format_description($md);
192 $html = file_get_contents('tests/plugins/resources/markdown.html'); 193 $html = file_get_contents('tests/plugins/resources/markdown.html');
193 194
194 $data = process_markdown($md); 195 $data = process_markdown(
196 $md,
197 $this->conf->get('security.markdown_escape', true),
198 $this->conf->get('security.allowed_protocols')
199 );
195 $this->assertEquals($html, $data); 200 $this->assertEquals($html, $data);
196 } 201 }
197 202
diff --git a/tests/plugins/resources/markdown.html b/tests/plugins/resources/markdown.html
index 07a5a32e..844a6f31 100644
--- a/tests/plugins/resources/markdown.html
+++ b/tests/plugins/resources/markdown.html
@@ -21,4 +21,13 @@
21next #foo</code></pre> 21next #foo</code></pre>
22<p>Block:</p> 22<p>Block:</p>
23<pre><code>lorem ipsum #foobar http://link.tld 23<pre><code>lorem ipsum #foobar http://link.tld
24#foobar http://link.tld</code></pre></div> \ No newline at end of file 24#foobar http://link.tld</code></pre>
25<p><a href="?123456">link</a><br />
26<img src="/img/train.png" alt="link" /><br />
27<a href="http://test.tld/path/?query=value#hash">link</a><br />
28<a href="http://test.tld/path/?query=value#hash">link</a><br />
29<a href="https://test.tld/path/?query=value#hash">link</a><br />
30<a href="ftp://test.tld/path/?query=value#hash">link</a><br />
31<a href="magnet:test.tld/path/?query=value#hash">link</a><br />
32<a href="http://alert('xss')">link</a><br />
33<a href="http://test.tld/path/?query=value#hash">link</a></p></div> \ No newline at end of file
diff --git a/tests/plugins/resources/markdown.md b/tests/plugins/resources/markdown.md
index 0b8be7c5..b8ebd934 100644
--- a/tests/plugins/resources/markdown.md
+++ b/tests/plugins/resources/markdown.md
@@ -21,4 +21,14 @@ Block:
21``` 21```
22lorem ipsum #foobar http://link.tld 22lorem ipsum #foobar http://link.tld
23#foobar http://link.tld 23#foobar http://link.tld
24``` \ No newline at end of file 24```
25
26[link](?123456)
27![link](/img/train.png)
28[link](test.tld/path/?query=value#hash)
29[link](http://test.tld/path/?query=value#hash)
30[link](https://test.tld/path/?query=value#hash)
31[link](ftp://test.tld/path/?query=value#hash)
32[link](magnet:test.tld/path/?query=value#hash)
33[link](javascript:alert('xss'))
34[link](other://test.tld/path/?query=value#hash) \ No newline at end of file