From ca8b01bcb3ad9cf5243e3c9d3f10a815f5918133 Mon Sep 17 00:00:00 2001 From: Allan Simon Date: Thu, 11 Apr 2024 16:28:08 +0000 Subject: [PATCH 1/2] Fix a XSS when using a custom format callable, it was silently bypassing twig default escape by wrapping the string in a "Markup" object that is whitelisted instead if people do need it we force them do to ->setStripTag(true) before --- src/Field/Configurator/CommonPostConfigurator.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Field/Configurator/CommonPostConfigurator.php b/src/Field/Configurator/CommonPostConfigurator.php index 532b236e9e..3a6017fe86 100644 --- a/src/Field/Configurator/CommonPostConfigurator.php +++ b/src/Field/Configurator/CommonPostConfigurator.php @@ -7,6 +7,7 @@ use EasyCorp\Bundle\EasyAdminBundle\Contracts\Field\FieldConfiguratorInterface; use EasyCorp\Bundle\EasyAdminBundle\Dto\EntityDto; use EasyCorp\Bundle\EasyAdminBundle\Dto\FieldDto; +use EasyCorp\Bundle\EasyAdminBundle\Field\TextField; use EasyCorp\Bundle\EasyAdminBundle\Provider\AdminContextProvider; use function Symfony\Component\String\u; use Twig\Markup; @@ -53,6 +54,14 @@ private function buildFormattedValueOption($value, FieldDto $field, EntityDto $e $formatted = $callable($field->getValue(), $entityDto->getInstance()); + // we don't want to unintentionally allow people to add XSS vulnerabilities + // in the code just because some people need to have HTML/JS + // so that if you want know what you're doing you have to explicitly + // disable this. + if ($field->getCustomOptions(TextField::OPTION_STRIP_TAGS)) { + return $formatted; + } + // if the callable returns a string, wrap it in a Twig Markup to render the // HTML and CSS/JS elements that it might contain return \is_string($formatted) ? new Markup($formatted, $this->charset) : $formatted; From 0c8aa0f31e426ee7366ae635314045e000a993b6 Mon Sep 17 00:00:00 2001 From: Allan Simon Date: Thu, 11 Apr 2024 16:52:15 +0000 Subject: [PATCH 2/2] Fix a XSS when using a custom format callable, it was silently bypassing twig default escape by wrapping the string in a "Markup" object that is whitelisted instead if people do need it we force them do to ->setStripTag(true) before --- src/Field/Configurator/CommonPostConfigurator.php | 4 ++-- src/Field/FieldTrait.php | 9 +++++++++ src/Field/TextField.php | 8 -------- src/Field/TextareaField.php | 9 --------- 4 files changed, 11 insertions(+), 19 deletions(-) diff --git a/src/Field/Configurator/CommonPostConfigurator.php b/src/Field/Configurator/CommonPostConfigurator.php index 3a6017fe86..0adbd60bdf 100644 --- a/src/Field/Configurator/CommonPostConfigurator.php +++ b/src/Field/Configurator/CommonPostConfigurator.php @@ -7,7 +7,7 @@ use EasyCorp\Bundle\EasyAdminBundle\Contracts\Field\FieldConfiguratorInterface; use EasyCorp\Bundle\EasyAdminBundle\Dto\EntityDto; use EasyCorp\Bundle\EasyAdminBundle\Dto\FieldDto; -use EasyCorp\Bundle\EasyAdminBundle\Field\TextField; +use EasyCorp\Bundle\EasyAdminBundle\Field\FieldTrait; use EasyCorp\Bundle\EasyAdminBundle\Provider\AdminContextProvider; use function Symfony\Component\String\u; use Twig\Markup; @@ -58,7 +58,7 @@ private function buildFormattedValueOption($value, FieldDto $field, EntityDto $e // in the code just because some people need to have HTML/JS // so that if you want know what you're doing you have to explicitly // disable this. - if ($field->getCustomOptions(TextField::OPTION_STRIP_TAGS)) { + if ($field->getCustomOption(FieldTrait::OPTION_STRIP_TAGS) ?? true) { return $formatted; } diff --git a/src/Field/FieldTrait.php b/src/Field/FieldTrait.php index 2d7ea5e791..258f4a632f 100644 --- a/src/Field/FieldTrait.php +++ b/src/Field/FieldTrait.php @@ -16,6 +16,8 @@ */ trait FieldTrait { + public const OPTION_STRIP_TAGS = 'stripTags'; + private FieldDto $dto; private function __construct() @@ -337,6 +339,13 @@ public function setCustomOptions(array $options): self return $this; } + + public function stripTags(bool $stripTags = true): self + { + $this->setCustomOption(self::OPTION_STRIP_TAGS, $stripTags); + + return $this; + } public function hideOnDetail(): self { diff --git a/src/Field/TextField.php b/src/Field/TextField.php index 3bb0314e18..09e00e749c 100644 --- a/src/Field/TextField.php +++ b/src/Field/TextField.php @@ -15,7 +15,6 @@ final class TextField implements FieldInterface public const OPTION_MAX_LENGTH = 'maxLength'; public const OPTION_RENDER_AS_HTML = 'renderAsHtml'; - public const OPTION_STRIP_TAGS = 'stripTags'; /** * @param TranslatableInterface|string|false|null $label @@ -55,11 +54,4 @@ public function renderAsHtml(bool $asHtml = true): self return $this; } - - public function stripTags(bool $stripTags = true): self - { - $this->setCustomOption(self::OPTION_STRIP_TAGS, $stripTags); - - return $this; - } } diff --git a/src/Field/TextareaField.php b/src/Field/TextareaField.php index 90c92b8e9e..014023b7e1 100644 --- a/src/Field/TextareaField.php +++ b/src/Field/TextareaField.php @@ -17,8 +17,6 @@ final class TextareaField implements FieldInterface public const OPTION_MAX_LENGTH = TextField::OPTION_MAX_LENGTH; public const OPTION_NUM_OF_ROWS = 'numOfRows'; public const OPTION_RENDER_AS_HTML = TextField::OPTION_RENDER_AS_HTML; - public const OPTION_STRIP_TAGS = TextField::OPTION_STRIP_TAGS; - /** * @param TranslatableInterface|string|false|null $label */ @@ -70,11 +68,4 @@ public function renderAsHtml(bool $asHtml = true): self return $this; } - - public function stripTags(bool $stripTags = true): self - { - $this->setCustomOption(self::OPTION_STRIP_TAGS, $stripTags); - - return $this; - } }