Skip to content

Commit b98cb48

Browse files
committed
fix(formatter): prevent stored XSS by disabling default HTML safety
Formatters no longer treat their output as HTML safe by default. This closes a stored XSS vector where unsanitized user input could inject script content. Existing formatters must now explicitly implement isHtmlSafe() to return true *and* ensure proper escaping/sanitization before claiming safety. BREAKING CHANGE: Default formatter behavior changed; outputs are now considered unsafe HTML unless explicitly marked safe. Audit custom formatter implementations.
1 parent d0584ca commit b98cb48

29 files changed

+71
-32
lines changed

.github/workflows/ci.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ jobs:
4545
SYMFONY_REQUIRE: ${{ matrix.symfony }}
4646
uses: ramsey/composer-install@v2
4747
- name: Run test suite on PHP ${{ matrix.php }} and Symfony ${{ matrix.symfony }}
48-
run: vendor/bin/simple-phpunit
48+
run: vendor/bin/phpunit
4949
- name: Run ECS
5050
run: vendor/bin/ecs
5151
- name: Run PHPStan

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,6 @@ phpstan:
3535

3636
## PHP Unit tests
3737
phpunit:
38-
vendor/bin/simple-phpunit
38+
vendor/bin/phpunit
3939

4040

composer.json

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
"symfony/translation": "^6.4|^7.0",
2828
"symfony/form": "^6.4|^7.0",
2929
"symfony/stopwatch": "^6.4|^7.0",
30-
"symfony/test-pack": "^1.1.0",
3130
"symfony/orm-pack": "^2.4.1"
3231
},
3332
"require-dev": {
@@ -38,7 +37,10 @@
3837
"doctrine/doctrine-bundle": "^2.5.5",
3938
"whatwedo/php-coding-standard": "^1.0",
4039
"symfony/twig-bundle": "^6.4|^7.0",
41-
"phpstan/phpstan": "^1.7"
40+
"phpstan/phpstan": "^1.7",
41+
"phpunit/phpunit": "^10",
42+
"symfony/test-pack": "^1.0",
43+
"slevomat/coding-standard": "8.22.1"
4244
},
4345
"autoload": {
4446
"psr-4": {

src/Formatter/AbstractFormatter.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
abstract class AbstractFormatter implements FormatterInterface
1010
{
11+
public const OPT_HTML_SAFE = 'html_safe';
12+
1113
/**
1214
* @var array<string, mixed>
1315
*/
@@ -34,7 +36,18 @@ public function processOptions(array $options = []): void
3436
$this->options = $resolver->resolve($options);
3537
}
3638

39+
public function isHtmlSafe(): bool
40+
{
41+
return (bool) ($this->options[self::OPT_HTML_SAFE] ?? false);
42+
}
43+
44+
protected function escapeHTML(string $value): string
45+
{
46+
return htmlspecialchars($value, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8');
47+
}
48+
3749
protected function configureOptions(OptionsResolver $resolver): void
3850
{
51+
$resolver->setDefault(self::OPT_HTML_SAFE, false);
3952
}
4053
}

src/Formatter/BadgeFormatter.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public function getHtml(mixed $value): string
2626
{
2727
$this->processOptions(($this->options[self::OPT_CONFIGURATION])($value, $this->options));
2828
return $this->twig->render($this->options[self::OPT_TEMPLATE], [
29-
'value' => $this->getString($value),
29+
'value' => $this->escapeHTML($this->getString($value)),
3030
'type' => $this->options[self::OPT_TYPE],
3131
'background_color_class' => $this->options[self::OPT_BACKGROUND_COLOR_CLASS],
3232
'background_color_hex' => $this->options[self::OPT_BACKGROUND_COLOR_HEX],
@@ -43,6 +43,7 @@ protected function configureOptions(OptionsResolver $resolver): void
4343
self::OPT_TYPE => 'neutral',
4444
self::OPT_LINK => null,
4545
self::OPT_CONFIGURATION => static fn (mixed $value, array $options): array => $options,
46+
self::OPT_HTML_SAFE => true,
4647
]);
4748
$resolver->setAllowedTypes(self::OPT_BACKGROUND_COLOR_CLASS, ['string', 'null']);
4849
$resolver->setAllowedTypes(self::OPT_BACKGROUND_COLOR_HEX, ['string', 'null']);

src/Formatter/CollectionFormatter.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
namespace araise\CoreBundle\Formatter;
66

77
use Doctrine\Common\Collections\Collection;
8+
use Symfony\Component\OptionsResolver\OptionsResolver;
89

910
class CollectionFormatter extends AbstractFormatter
1011
{
@@ -29,9 +30,15 @@ public function getHtml(mixed $value): string
2930
}
3031
$str = '<ul>';
3132
foreach ($value as $singleValue) {
32-
$str .= '<li>'.$singleValue.'</li>';
33+
$str .= '<li>'.$this->escapeHTML($singleValue).'</li>';
3334
}
3435

3536
return $str.'</ul>';
3637
}
38+
39+
protected function configureOptions(OptionsResolver $resolver): void
40+
{
41+
parent::configureOptions($resolver);
42+
$resolver->setDefault(self::OPT_HTML_SAFE, true);
43+
}
3744
}

src/Formatter/EmailFormatter.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@ public function getHtml(mixed $value): string
2727

2828
return $value ? sprintf(
2929
'<a href="mailto:%s" title="%s">%s</a>',
30-
$value,
31-
$title,
32-
$value
30+
$this->escapeHTML($value),
31+
$this->escapeHTML($title),
32+
$this->escapeHTML($value)
3333
) : '';
3434
}
3535
}

src/Formatter/Nl2brFormatter.php

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
namespace araise\CoreBundle\Formatter;
66

7+
use Symfony\Component\OptionsResolver\OptionsResolver;
8+
79
class Nl2brFormatter extends AbstractFormatter
810
{
911
public function getString(mixed $value): string
@@ -13,6 +15,12 @@ public function getString(mixed $value): string
1315

1416
public function getHtml(mixed $value): string
1517
{
16-
return nl2br($value);
18+
return nl2br($this->escapeHTML($value));
19+
}
20+
21+
protected function configureOptions(OptionsResolver $resolver): void
22+
{
23+
parent::configureOptions($resolver);
24+
$resolver->setDefault(self::OPT_HTML_SAFE, true);
1725
}
1826
}

src/Formatter/TwigFormatter.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,5 +57,6 @@ protected function configureOptions(OptionsResolver $resolver): void
5757
{
5858
$resolver->setRequired(self::OPT_TEMPLATE);
5959
$resolver->setAllowedTypes(self::OPT_TEMPLATE, 'string');
60+
$resolver->setDefault(self::OPT_HTML_SAFE, true);
6061
}
6162
}

src/Formatter/WysiwygFormatter.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929

3030
namespace araise\CoreBundle\Formatter;
3131

32+
use Symfony\Component\OptionsResolver\OptionsResolver;
33+
3234
class WysiwygFormatter extends AbstractFormatter
3335
{
3436
public function getString(mixed $value): string
@@ -42,4 +44,10 @@ public function getHtml(mixed $value): string
4244

4345
return $value ? sprintf('<blockquote>%s</blockquote>', $value) : '';
4446
}
47+
48+
protected function configureOptions(OptionsResolver $resolver): void
49+
{
50+
parent::configureOptions($resolver);
51+
$resolver->setDefault(self::OPT_HTML_SAFE, true);
52+
}
4553
}

0 commit comments

Comments
 (0)