Knah-Tsaeb/MyShaarli
Knah-Tsaeb/MyShaarli
1
0
Fork 0
Code Issues Pull requests Releases Wiki Activity

Add markdown_escape setting

Browse source 
This setting allows to escape HTML in markdown rendering or not.
The goal behind it is to avoid XSS issue in shared instances.

More info:

  * the setting is set to true by default
  * it is set to false for anyone who already have the plugin enabled
  (avoid breaking existing entries)
  * improve the HTML sanitization when the setting is set to false - but don't consider it XSS proof
  * mention the setting in the plugin README
This commit is contained in:
ArthurHoaro 2017-02-27 19:45:55 +01:00
parent 5978588578
commit e037610115
6 changed files with 181 additions and 27 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

23
application/Updater.php
View file

@ -336,6 +336,29 @@ public function updateMethodDefaultThemeVintage()
}
$this->conf->set('resource.theme', 'vintage');
$this->conf->write($this->isLoggedIn);
return true;
}
/**
* * `markdown_escape` is a new setting, set to true as default.
*
* If the markdown plugin was already enabled, escaping is disabled to avoid
* breaking existing entries.
*/
public function updateMethodEscapeMarkdown()
{
if ($this->conf->exists('security.markdown_escape')) {
return true;
}
if (in_array('markdown', $this->conf->get('general.enabled_plugins'))) {
$this->conf->set('security.markdown_escape', false);
} else {
$this->conf->set('security.markdown_escape', true);
}
$this->conf->write($this->isLoggedIn);
return true;
}
}

27
plugins/markdown/README.md
View file

@ -50,9 +50,20 @@ If the tag `nomarkdown` is set for a shaare, it won't be converted to Markdown s
> Note: this is a special tag, so it won't be displayed in link list.
### HTML rendering
### HTML escape
Markdown support HTML tags. For example:
By default, HTML tags are escaped. You can enable HTML tags rendering
by setting `security.markdwon_escape` to `false` in `data/config.json.php`:
```json
{
"security": {
"markdown_escape": false
}
}
```
With this setting, Markdown support HTML tags. For example:
> <strong>strong</strong><strike>strike</strike>
@ -60,12 +71,14 @@ Will render as:
> <strong>strong</strong><strike>strike</strike>
If you want to shaare HTML code, it is necessary to use inline code or code blocks.
**If your shaared descriptions containing HTML tags before enabling the markdown plugin,
enabling it might break your page.**
> Note: HTML tags such as script, iframe, etc. are disabled for security reasons.
**Warning:**
* This setting might present **security risks** (XSS) on shared instances, even though tags
such as script, iframe, etc should be disabled.
* If you want to shaare HTML code, it is necessary to use inline code or code blocks.
* If your shaared descriptions contained HTML tags before enabling the markdown plugin,
enabling it might break your page.
### Known issue

29
plugins/markdown/markdown.php
View file

@ -14,18 +14,19 @@
/**
* Parse linklist descriptions.
*
* @param array $data linklist data.
* @param array $data linklist data.
* @param ConfigManager $conf instance.
*
* @return mixed linklist data parsed in markdown (and converted to HTML).
*/
function hook_markdown_render_linklist($data)
function hook_markdown_render_linklist($data, $conf)
{
foreach ($data['links'] as &$value) {
if (!empty($value['tags']) && noMarkdownTag($value['tags'])) {
$value = stripNoMarkdownTag($value);
continue;
}
$value['description'] = process_markdown($value['description']);
$value['description'] = process_markdown($value['description'], $conf->get('security.markdown_escape', true));
}
return $data;
}
@ -34,17 +35,18 @@ function hook_markdown_render_linklist($data)
* Parse feed linklist descriptions.
*
* @param array $data linklist data.
* @param ConfigManager $conf instance.
*
* @return mixed linklist data parsed in markdown (and converted to HTML).
*/
function hook_markdown_render_feed($data)
function hook_markdown_render_feed($data, $conf)
{
foreach ($data['links'] as &$value) {
if (!empty($value['tags']) && noMarkdownTag($value['tags'])) {
$value = stripNoMarkdownTag($value);
continue;
}
$value['description'] = process_markdown($value['description']);
$value['description'] = process_markdown($value['description'], $conf->get('security.markdown_escape', true));
}
return $data;
@ -53,11 +55,12 @@ function hook_markdown_render_feed($data)
/**
* Parse daily descriptions.
*
* @param array $data daily data.
* @param array $data daily data.
* @param ConfigManager $conf instance.
*
* @return mixed daily data parsed in markdown (and converted to HTML).
*/
function hook_markdown_render_daily($data)
function hook_markdown_render_daily($data, $conf)
{
// Manipulate columns data
foreach ($data['cols'] as &$value) {
@ -66,7 +69,10 @@ function hook_markdown_render_daily($data)
$value2 = stripNoMarkdownTag($value2);
continue;
}
$value2['formatedDescription'] = process_markdown($value2['formatedDescription']);
$value2['formatedDescription'] = process_markdown(
$value2['formatedDescription'],
$conf->get('security.markdown_escape', true)
);
}
}
@ -250,7 +256,7 @@ function ($match) { return escape($match[0]); },
$description);
}
$description = preg_replace(
'#(<[^>]+)on[a-z]*="[^"]*"#is',
'#(<[^>]+)on[a-z]*="?[^ "]*"?#is',
'$1',
$description);
return $description;
@ -265,10 +271,11 @@ function ($match) { return escape($match[0]); },
* 5. Wrap description in 'markdown' CSS class.
*
* @param string $description input description text.
* @param bool $escape escape HTML entities
*
* @return string HTML processed $description.
*/
function process_markdown($description)
function process_markdown($description, $escape = true)
{
$parsedown = new Parsedown();
@ -278,7 +285,7 @@ function process_markdown($description)
$processedDescription = reverse_text2clickable($processedDescription);
$processedDescription = unescape($processedDescription);
$processedDescription = $parsedown
->setMarkupEscaped(false)
->setMarkupEscaped($escape)
->setBreaksEnabled(true)
->text($processedDescription);
$processedDescription = sanitize_html($processedDescription);

66
tests/Updater/UpdaterTest.php
View file

@ -506,4 +506,70 @@ public function testSetDefaultThemeNothingToDo()
$this->conf = new ConfigManager($sandboxConf);
$this->assertEquals($theme, $this->conf->get('resource.theme'));
}
/**
* Test updateMethodEscapeMarkdown with markdown plugin enabled
* => setting markdown_escape set to false.
*/
public function testEscapeMarkdownSettingToFalse()
{
$sandboxConf = 'sandbox/config';
copy(self::$configFile . '.json.php', $sandboxConf . '.json.php');
$this->conf = new ConfigManager($sandboxConf);
$this->conf->set('general.enabled_plugins', ['markdown']);
$updater = new Updater([], [], $this->conf, true);
$this->assertTrue($updater->updateMethodEscapeMarkdown());
$this->assertFalse($this->conf->get('security.markdown_escape'));
// reload from file
$this->conf = new ConfigManager($sandboxConf);
$this->assertFalse($this->conf->get('security.markdown_escape'));
}
/**
* Test updateMethodEscapeMarkdown with markdown plugin disabled
* => setting markdown_escape set to true.
*/
public function testEscapeMarkdownSettingToTrue()
{
$sandboxConf = 'sandbox/config';
copy(self::$configFile . '.json.php', $sandboxConf . '.json.php');
$this->conf = new ConfigManager($sandboxConf);
$this->conf->set('general.enabled_plugins', []);
$updater = new Updater([], [], $this->conf, true);
$this->assertTrue($updater->updateMethodEscapeMarkdown());
$this->assertTrue($this->conf->get('security.markdown_escape'));
// reload from file
$this->conf = new ConfigManager($sandboxConf);
$this->assertTrue($this->conf->get('security.markdown_escape'));
}
/**
* Test updateMethodEscapeMarkdown with nothing to do (setting already enabled)
*/
public function testEscapeMarkdownSettingNothingToDoEnabled()
{
$sandboxConf = 'sandbox/config';
copy(self::$configFile . '.json.php', $sandboxConf . '.json.php');
$this->conf = new ConfigManager($sandboxConf);
$this->conf->set('security.markdown_escape', true);
$updater = new Updater([], [], $this->conf, true);
$this->assertTrue($updater->updateMethodEscapeMarkdown());
$this->assertTrue($this->conf->get('security.markdown_escape'));
}
/**
* Test updateMethodEscapeMarkdown with nothing to do (setting already disabled)
*/
public function testEscapeMarkdownSettingNothingToDoDisabled()
{
$this->conf->set('security.markdown_escape', false);
$updater = new Updater([], [], $this->conf, true);
$this->assertTrue($updater->updateMethodEscapeMarkdown());
$this->assertFalse($this->conf->get('security.markdown_escape'));
}
}

57
tests/plugins/PluginMarkdownTest.php
View file

@ -13,12 +13,18 @@
*/
class PluginMarkdownTest extends PHPUnit_Framework_TestCase
{
/**
* @var ConfigManager instance.
*/
protected $conf;
/**
* Reset plugin path
*/
public function setUp()
{
PluginManager::$PLUGINS_PATH = 'plugins';
$this->conf = new ConfigManager('tests/utils/config/configJson');
}
/**
@ -36,7 +42,7 @@ public function testMarkdownLinklist()
),
);
$data = hook_markdown_render_linklist($data);
$data = hook_markdown_render_linklist($data, $this->conf);
$this->assertNotFalse(strpos($data['links'][0]['description'], '<h1>'));
$this->assertNotFalse(strpos($data['links'][0]['description'], '<p>'));
}
@ -61,7 +67,7 @@ public function testMarkdownDaily()
),
);
$data = hook_markdown_render_daily($data);
$data = hook_markdown_render_daily($data, $this->conf);
$this->assertNotFalse(strpos($data['cols'][0][0]['formatedDescription'], '<h1>'));
$this->assertNotFalse(strpos($data['cols'][0][0]['formatedDescription'], '<p>'));
}
@ -110,6 +116,8 @@ public function testSanitizeHtml()
$output = escape($input);
$input .= '<a href="#" onmouseHover="alert(\'xss\');" attr="tt">link</a>';
$output .= '<a href="#" attr="tt">link</a>';
$input .= '<a href="#" onmouseHover=alert(\'xss\'); attr="tt">link</a>';
$output .= '<a href="#" attr="tt">link</a>';
$this->assertEquals($output, sanitize_html($input));
// Do not touch escaped HTML.
$input = escape($input);
@ -130,10 +138,10 @@ public function testNoMarkdownTag()
))
);
$processed = hook_markdown_render_linklist($data);
$processed = hook_markdown_render_linklist($data, $this->conf);
$this->assertEquals($str, $processed['links'][0]['description']);
$processed = hook_markdown_render_feed($data);
$processed = hook_markdown_render_feed($data, $this->conf);
$this->assertEquals($str, $processed['links'][0]['description']);
$data = array(
@ -151,7 +159,7 @@ public function testNoMarkdownTag()
),
);
$data = hook_markdown_render_daily($data);
$data = hook_markdown_render_daily($data, $this->conf);
$this->assertEquals($str, $data['cols'][0][0]['formatedDescription']);
}
@ -169,7 +177,7 @@ public function testNoMarkdownNotExcactlyMatching()
))
);
$data = hook_markdown_render_feed($data);
$data = hook_markdown_render_feed($data, $this->conf);
$this->assertContains('<em>', $data['links'][0]['description']);
}
@ -185,4 +193,41 @@ public function testMarkdownHashtagLinks()
$data = process_markdown($md);
$this->assertEquals($html, $data);
}
/**
* Make sure that the HTML tags are escaped.
*/
public function testMarkdownWithHtmlEscape()
{
$md = '**strong** <strong>strong</strong>';
$html = '<div class="markdown"><p><strong>strong</strong> &lt;strong&gt;strong&lt;/strong&gt;</p></div>';
$data = array(
'links' => array(
0 => array(
'description' => $md,
),
),
);
$data = hook_markdown_render_linklist($data, $this->conf);
$this->assertEquals($html, $data['links'][0]['description']);
}
/**
* Make sure that the HTML tags aren't escaped with the setting set to false.
*/
public function testMarkdownWithHtmlNoEscape()
{
$this->conf->set('security.markdown_escape', false);
$md = '**strong** <strong>strong</strong>';
$html = '<div class="markdown"><p><strong>strong</strong> <strong>strong</strong></p></div>';
$data = array(
'links' => array(
0 => array(
'description' => $md,
),
),
);
$data = hook_markdown_render_linklist($data, $this->conf);
$this->assertEquals($html, $data['links'][0]['description']);
}
}

6
tests/plugins/resources/markdown.html
View file

@ -12,11 +12,11 @@
<li><a href="http://link.tld">two</a></li>
<li><a href="http://link.tld">three</a></li>
<li><a href="http://link.tld">four</a></li>
<li>foo <a href="?addtag=foobar" title="Hashtag foobar">#foobar</a></li>
<li>foo &lt;a href=&quot;?addtag=foobar&quot; title=&quot;Hashtag foobar&quot;&gt;#foobar&lt;/a&gt;</li>
</ol></li>
</ol>
<p><a href="?addtag=foobar" title="Hashtag foobar">#foobar</a> foo <code>lol #foo</code> <a href="?addtag=bar" title="Hashtag bar">#bar</a></p>
<p>fsdfs <a href="http://link.tld">http://link.tld</a> <a href="?addtag=foobar" title="Hashtag foobar">#foobar</a> <code>http://link.tld</code></p>
<p>&lt;a href=&quot;?addtag=foobar&quot; title=&quot;Hashtag foobar&quot;&gt;#foobar&lt;/a&gt; foo <code>lol #foo</code> &lt;a href=&quot;?addtag=bar&quot; title=&quot;Hashtag bar&quot;&gt;#bar&lt;/a&gt;</p>
<p>fsdfs <a href="http://link.tld">http://link.tld</a> &lt;a href=&quot;?addtag=foobar&quot; title=&quot;Hashtag foobar&quot;&gt;#foobar&lt;/a&gt; <code>http://link.tld</code></p>
<pre><code>http://link.tld #foobar
next #foo</code></pre>
<p>Block:</p>