diff --git a/application/Url.php b/application/Url.php
index 25a62a8a..b3759377 100644
--- a/application/Url.php
+++ b/application/Url.php
@@ -63,6 +63,30 @@ function add_trailing_slash($url)
return $url . (!endsWith($url, '/') ? '/' : '');
}
+/**
+ * Replace not whitelisted protocols by 'http://' from given URL.
+ *
+ * @param string $url URL to clean
+ * @param array $protocols List of allowed protocols (aside from http(s)).
+ *
+ * @return string URL with allowed protocol
+ */
+function whitelist_protocols($url, $protocols)
+{
+ if (startsWith($url, '?') || startsWith($url, '/')) {
+ return $url;
+ }
+ $protocols = array_merge(['http', 'https'], $protocols);
+ $protocol = preg_match('#^(\w+):/?/?#', $url, $match);
+ // Protocol not allowed: we remove it and replace it with http
+ if ($protocol === 1 && ! in_array($match[1], $protocols)) {
+ $url = str_replace($match[0], 'http://', $url);
+ } else if ($protocol !== 1) {
+ $url = 'http://' . $url;
+ }
+ return $url;
+}
+
/**
* URL representation and cleanup utilities
*
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 @@ protected function setDefaultValues()
$this->setEmpty('security.ban_duration', 1800);
$this->setEmpty('security.session_protection_disabled', false);
$this->setEmpty('security.open_shaarli', false);
+ $this->setEmpty('security.allowed_protocols', ['ftp', 'ftps', 'magnet']);
$this->setEmpty('general.header_link', '?');
$this->setEmpty('general.links_per_page', 20);
diff --git a/index.php b/index.php
index 468dd091..944af674 100644
--- a/index.php
+++ b/index.php
@@ -1237,13 +1237,7 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history)
// Remove duplicates.
$tags = implode(' ', array_unique(explode(' ', $tags)));
- $url = trim($_POST['lf_url']);
- if (! startsWith($url, 'http:') && ! startsWith($url, 'https:')
- && ! startsWith($url, 'ftp:') && ! startsWith($url, 'magnet:')
- && ! startsWith($url, '?') && ! startsWith($url, 'javascript:')
- ) {
- $url = 'http://' . $url;
- }
+ $url = whitelist_protocols(trim($_POST['lf_url']), $conf->get('security.allowed_protocols'));
$link = array(
'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)
$value = stripNoMarkdownTag($value);
continue;
}
- $value['description'] = process_markdown($value['description'], $conf->get('security.markdown_escape', true));
+ $value['description'] = process_markdown(
+ $value['description'],
+ $conf->get('security.markdown_escape', true),
+ $conf->get('security.allowed_protocols')
+ );
}
return $data;
}
@@ -46,7 +50,11 @@ function hook_markdown_render_feed($data, $conf)
$value = stripNoMarkdownTag($value);
continue;
}
- $value['description'] = process_markdown($value['description'], $conf->get('security.markdown_escape', true));
+ $value['description'] = process_markdown(
+ $value['description'],
+ $conf->get('security.markdown_escape', true),
+ $conf->get('security.allowed_protocols')
+ );
}
return $data;
@@ -71,7 +79,8 @@ function hook_markdown_render_daily($data, $conf)
}
$value2['formatedDescription'] = process_markdown(
$value2['formatedDescription'],
- $conf->get('security.markdown_escape', true)
+ $conf->get('security.markdown_escape', true),
+ $conf->get('security.allowed_protocols')
);
}
}
@@ -231,6 +240,25 @@ function reverse_space2nbsp($description)
return preg_replace('/(^| ) /m', '$1 ', $description);
}
+/**
+ * Replace not whitelisted protocols with http:// in given description.
+ *
+ * @param string $description input description text.
+ * @param array $allowedProtocols list of allowed protocols.
+ *
+ * @return string $description without malicious link.
+ */
+function filter_protocols($description, $allowedProtocols)
+{
+ return preg_replace_callback(
+ '#]\((.*?)\)#is',
+ function ($match) use ($allowedProtocols) {
+ return ']('. whitelist_protocols($match[1], $allowedProtocols) .')';
+ },
+ $description
+ );
+}
+
/**
* Remove dangerous HTML tags (tags, iframe, etc.).
* Doesn't affect content (already escaped by Parsedown).
@@ -275,7 +303,7 @@ function ($match) { return escape($match[0]); },
*
* @return string HTML processed $description.
*/
-function process_markdown($description, $escape = true)
+function process_markdown($description, $escape = true, $allowedProtocols = [])
{
$parsedown = new Parsedown();
@@ -283,6 +311,7 @@ function process_markdown($description, $escape = true)
$processedDescription = reverse_nl2br($processedDescription);
$processedDescription = reverse_space2nbsp($processedDescription);
$processedDescription = reverse_text2clickable($processedDescription);
+ $processedDescription = filter_protocols($processedDescription, $allowedProtocols);
$processedDescription = unescape($processedDescription);
$processedDescription = $parsedown
->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 @@
+assertEquals($url, whitelist_protocols($url, $whitelist));
+ $url = '/path.jpg';
+ $this->assertEquals($url, whitelist_protocols($url, $whitelist));
+ }
+
+ /**
+ * Test whitelist_protocols() on a note (relative URL).
+ */
+ public function testWhitelistProtocolMissing()
+ {
+ $whitelist = ['ftp', 'magnet'];
+ $url = 'test.tld/path/?query=value#hash';
+ $this->assertEquals('http://'. $url, whitelist_protocols($url, $whitelist));
+ }
+
+ /**
+ * Test whitelist_protocols() with allowed protocols.
+ */
+ public function testWhitelistAllowedProtocol()
+ {
+ $whitelist = ['ftp', 'magnet'];
+ $url = 'http://test.tld/path/?query=value#hash';
+ $this->assertEquals($url, whitelist_protocols($url, $whitelist));
+ $url = 'https://test.tld/path/?query=value#hash';
+ $this->assertEquals($url, whitelist_protocols($url, $whitelist));
+ $url = 'ftp://test.tld/path/?query=value#hash';
+ $this->assertEquals($url, whitelist_protocols($url, $whitelist));
+ $url = 'magnet:test.tld/path/?query=value#hash';
+ $this->assertEquals($url, whitelist_protocols($url, $whitelist));
+ }
+
+ /**
+ * Test whitelist_protocols() with allowed protocols.
+ */
+ public function testWhitelistDisallowedProtocol()
+ {
+ $whitelist = ['ftp', 'magnet'];
+ $url = 'javascript:alert("xss");';
+ $this->assertEquals('http://alert("xss");', whitelist_protocols($url, $whitelist));
+ $url = 'other://test.tld/path/?query=value#hash';
+ $this->assertEquals('http://test.tld/path/?query=value#hash', whitelist_protocols($url, $whitelist));
+ }
+}
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 @@ public function setUp()
{
PluginManager::$PLUGINS_PATH = 'plugins';
$this->conf = new ConfigManager('tests/utils/config/configJson');
+ $this->conf->set('security.allowed_protocols', ['ftp', 'magnet']);
}
/**
@@ -183,15 +184,19 @@ public function testNoMarkdownNotExcactlyMatching()
}
/**
- * Test hashtag links processed with markdown.
+ * Make sure that the generated HTML match the reference HTML file.
*/
- public function testMarkdownHashtagLinks()
+ public function testMarkdownGlobalProcessDescription()
{
$md = file_get_contents('tests/plugins/resources/markdown.md');
$md = format_description($md);
$html = file_get_contents('tests/plugins/resources/markdown.html');
- $data = process_markdown($md);
+ $data = process_markdown(
+ $md,
+ $this->conf->get('security.markdown_escape', true),
+ $this->conf->get('security.allowed_protocols')
+ );
$this->assertEquals($html, $data);
}
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 @@
next #foo
Block:
lorem ipsum #foobar http://link.tld
-#foobar http://link.tld
\ No newline at end of file
+#foobar http://link.tld
+
\ 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:
```
lorem ipsum #foobar http://link.tld
#foobar http://link.tld
-```
\ No newline at end of file
+```
+
+[link](?123456)
+![link](/img/train.png)
+[link](test.tld/path/?query=value#hash)
+[link](http://test.tld/path/?query=value#hash)
+[link](https://test.tld/path/?query=value#hash)
+[link](ftp://test.tld/path/?query=value#hash)
+[link](magnet:test.tld/path/?query=value#hash)
+[link](javascript:alert('xss'))
+[link](other://test.tld/path/?query=value#hash)
\ No newline at end of file