Skip to content

Commit

Permalink
Sanitize headline (#1550)
Browse files Browse the repository at this point in the history
| Q             | A
| ------------- | ---
| Bug fix?      | yes
| Backport      | 0.9, 0.10
| Tickets       | #1543
| License       | MIT

According to #1543, there is a security issue where xss code can be executed in frontend. Enhavo already provide a twig filter `html_sanitize` to prevent xss injections, but in this case the filter is not applied to the output of the `headline` filter.  The `headline` filter is marked as html safe, which is not true. To unmark it as html safe will cause bc breaks because the output will be encoded html. So we apply the sanitize routine to the headline filter to satisfy the html safe mark.

Further outputs where checked as well and the `raw` filter was replaced with `html_sanitize` where html output from the user is expected.
  • Loading branch information
gseidel committed May 2, 2022
1 parent 6ec7821 commit 33c68ba
Show file tree
Hide file tree
Showing 6 changed files with 13 additions and 7 deletions.
Expand Up @@ -7,7 +7,7 @@
<div class="article-content">
<div class="article-date">{{ resource.publicationDate|date('d.m.y') }}</div>
<div class="article-title">{{ resource.title }}</div>
<div class="article-teaser">{{ resource.teaser|raw }}</div>
<div class="article-teaser">{{ resource.teaser|html_sanitize }}</div>
{# <a href="{{ router(resource) }}" class="btn">Read more</a>#}
</div>
</article>
Expand Down
Expand Up @@ -11,6 +11,10 @@ class HtmlSanitizerTest extends TestCase
public function testFormatHeadline()
{
$sanitizer = $this->getMockBuilder(HtmlSanitizer::class)->disableOriginalConstructor()->getMock();
$sanitizer->expects($this->atLeast(1))->method('sanitize')->willReturnCallback(function ($value) {
return $value;
});

$extension = new FormatExtension($sanitizer);

$this->assertEquals(
Expand Down
6 changes: 4 additions & 2 deletions src/Enhavo/Bundle/FormBundle/Twig/FormatExtension.php
Expand Up @@ -78,12 +78,14 @@ public function formatHeadline($value, $class = '', array $attributes = [])

$pattern = '/^<([a-zA-Z0-9-]+)>/';
if(preg_match($pattern, $value)) {
return preg_replace_callback($pattern, function($matches) use ($attribute) {
$content = preg_replace_callback($pattern, function($matches) use ($attribute) {
return sprintf('<%s%s>', $matches[1], $attribute);
}, $value);
} else {
return sprintf('<div%s>%s</div>', $attribute, $value);
$content = sprintf('<div%s>%s</div>', $attribute, $value);
}

return $this->sanitizer->sanitize($content);
}

public function sanitizeHtml($value, $options = [])
Expand Down
Expand Up @@ -5,8 +5,8 @@
<div data-id={{ item.resource.id }} {% if is_granted('WORKFLOW_UPDATE', item.resource) %}data-update-route="{{ item.resource|updateRoute }}"{% endif %} class="entry-row">
<div class="row">
<div class="col-xs-9">
<h1> {{ item.resource.title|raw }} </h1>
{{ item.highlightedText|raw }}
<h1> {{ item.resource.title|html_sanitize }} </h1>
{{ item.highlightedText|html_sanitize }}
</div>
</div>
</div>
Expand Down
Expand Up @@ -11,7 +11,7 @@
<div class="row">
<div class="col-xs-12">
<h1>{{ result.title }}</h1>
{{ result.text|raw }}
{{ result.text|html_sanitize }}
<a href="{{ router(result.subject) }}">{{ result.title }}</a>
</div>
</div>
Expand Down
Expand Up @@ -8,7 +8,7 @@
{% endif %}
<span class="container hero-container">
<span class="hero-caption" href="">
<p class="slide-text">{{ slide.text|raw }}</p>
<p class="slide-text">{{ slide.text|html_sanitize }}</p>
<h1 class="slide-title">{{ slide.title }}</h1>
</span>
</span>
Expand Down

0 comments on commit 33c68ba

Please sign in to comment.