Skip to content

Commit dad9934

Browse files
javiereguiluzjaviereguiluz
committed
bug #7551 Improve the security of the Url field (javiereguiluz)
This PR was merged into the 4.x branch. Discussion ---------- Improve the security of the Url field This was spotted by a security analysis made by Claude. It is related to security issues CWE-79 (XSS), CWE-601 (related: open redirect), CWE-184 (Incomplete Disallow-List of URL schemes). Summary _(I removed most of the details and exploitable steps)_: EasyAdmin's built-in `UrlField` renders its value directly into an HTML `href` attribute on both the list and detail pages. Twig auto-escaping turns quotes into `&quot;` but does not filter URI schemes — values such as `javascript:alert(document.cookie)` are stored verbatim and rendered as a live, clickable `<a href="javascript:…">`. Commits ------- 397253f Improve the security of the Url field
2 parents 63e46c6 + 397253f commit dad9934

5 files changed

Lines changed: 321 additions & 4 deletions

File tree

‎doc/fields/UrlField.rst‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,37 @@ Basic Information
2323
Options
2424
-------
2525

26-
This field does not define any custom option.
26+
allowedProtocols
27+
~~~~~~~~~~~~~~~~
28+
29+
Restricts the protocols accepted as valid input by attaching a Symfony
30+
``Url`` constraint to the form field. Pass the protocols without the
31+
trailing colon::
32+
33+
UrlField::new('homepage')
34+
->allowedProtocols(['http', 'https']);
35+
36+
By default no protocol restriction is applied, so any scheme-looking
37+
value (like ``ftp://``, ``ssh://`` or ``mailto:``) is accepted. Use this
38+
option when you know the field should only store web URLs.
39+
40+
Regardless of this option, EasyAdmin always renders known-dangerous
41+
schemes (``javascript:``, ``data:``, ``vbscript:`` and ``file:``) as
42+
plain text instead of clickable links, to prevent stored XSS attacks in
43+
the backend.
44+
45+
setDefaultProtocol
46+
~~~~~~~~~~~~~~~~~~
47+
48+
Defines the protocol prepended to the submitted value when it doesn't
49+
include one. If you don't set it, no protocol is prepended and the field
50+
is rendered using an ``<input type="url">`` element so the browser
51+
validates the value::
52+
53+
UrlField::new('homepage')
54+
->setDefaultProtocol('https');
55+
56+
See the `UrlType default_protocol option`_ for details.
2757

2858
.. _`UrlType`: https://symfony.com/doc/current/reference/forms/types/url.html
59+
.. _`UrlType default_protocol option`: https://symfony.com/doc/current/reference/forms/types/url.html#default-protocol

‎src/Field/Configurator/UrlConfigurator.php‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,23 @@
88
use EasyCorp\Bundle\EasyAdminBundle\Dto\EntityDto;
99
use EasyCorp\Bundle\EasyAdminBundle\Dto\FieldDto;
1010
use EasyCorp\Bundle\EasyAdminBundle\Field\UrlField;
11+
use Symfony\Component\Validator\Constraints\Url;
1112
use function Symfony\Component\String\u;
1213

1314
/**
1415
* @author Javier Eguiluz <javier.eguiluz@gmail.com>
1516
*/
1617
final class UrlConfigurator implements FieldConfiguratorInterface
1718
{
19+
/**
20+
* These schemes are considered dangerous and make the URL to not be rendered as a clickable link:
21+
*
22+
* `javascript:` - executes script in the document's origin on click.
23+
* `data:` - `data:text/html,...` renders attacker HTML (and inline script) in the clicked tab.
24+
* `vbscript:` - legacy IE / some embedded browsers execute VBScript.
25+
*/
26+
private const DANGEROUS_SCHEMES = ['javascript', 'data', 'vbscript', 'file'];
27+
1828
public function supports(FieldDto $field, EntityDto $entityDto): bool
1929
{
2030
return UrlField::class === $field->getFieldFqcn();
@@ -25,7 +35,24 @@ public function configure(FieldDto $field, EntityDto $entityDto, AdminContext $c
2535
$field->setFormTypeOptionIfNotSet('attr.inputmode', 'url');
2636
$field->setFormTypeOptionIfNotSet('default_protocol', $field->getCustomOption(UrlField::OPTION_DEFAULT_PROTOCOL));
2737

28-
$prettyUrl = str_replace(['http://www.', 'https://www.', 'http://', 'https://'], '', (string) $field->getValue());
38+
$allowedProtocols = $field->getCustomOption(UrlField::OPTION_ALLOWED_PROTOCOLS);
39+
if (\is_array($allowedProtocols)) {
40+
$constraints = $field->getFormTypeOption('constraints') ?? [];
41+
$constraints[] = new Url(protocols: $allowedProtocols);
42+
$field->setFormTypeOption('constraints', $constraints);
43+
}
44+
45+
$url = (string) $field->getValue();
46+
// browsers strip leading whitespace and control bytes before parsing the scheme,
47+
// so the same normalization must happen here before matching against the denylist
48+
$normalizedUrl = ltrim($url, "\0..\x20");
49+
50+
// scheme characters per RFC 3986: scheme = `ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )`
51+
$isUnsafe = 1 === preg_match('#^([a-z][a-z0-9+.\-]*):#i', $normalizedUrl, $matches)
52+
&& \in_array(strtolower($matches[1]), self::DANGEROUS_SCHEMES, true);
53+
$field->setCustomOption(UrlField::OPTION_IS_UNSAFE, $isUnsafe);
54+
55+
$prettyUrl = str_replace(['http://www.', 'https://www.', 'http://', 'https://'], '', $url);
2956
$prettyUrl = rtrim($prettyUrl, '/');
3057

3158
if (Action::INDEX === $context->getCrud()->getCurrentAction()) {

‎src/Field/UrlField.php‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ final class UrlField implements FieldInterface
1414
use FieldTrait;
1515

1616
public const OPTION_DEFAULT_PROTOCOL = 'defaultProtocol';
17+
public const OPTION_IS_UNSAFE = 'isUnsafe';
18+
public const OPTION_ALLOWED_PROTOCOLS = 'allowedProtocols';
1719

1820
/**
1921
* @param TranslatableInterface|string|false|null $label
@@ -27,7 +29,8 @@ public static function new(string $propertyName, $label = null): self
2729
->setFormType(UrlType::class)
2830
->addCssClass('field-url')
2931
->setDefaultColumns('col-md-10 col-xxl-8')
30-
->setCustomOption(self::OPTION_DEFAULT_PROTOCOL, null);
32+
->setCustomOption(self::OPTION_DEFAULT_PROTOCOL, null)
33+
->setCustomOption(self::OPTION_ALLOWED_PROTOCOLS, null);
3134
}
3235

3336
/**
@@ -42,4 +45,18 @@ public function setDefaultProtocol(string $protocol): self
4245

4346
return $this;
4447
}
48+
49+
/**
50+
* Restricts the protocols accepted as valid form input via a
51+
* Symfony Url constraint. Pass protocols without the trailing colon
52+
* (e.g. ['http', 'https']).
53+
*
54+
* @param string[] $protocols
55+
*/
56+
public function allowedProtocols(array $protocols): self
57+
{
58+
$this->setCustomOption(self::OPTION_ALLOWED_PROTOCOLS, $protocols);
59+
60+
return $this;
61+
}
4562
}

‎templates/crud/field/url.html.twig‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
{# @var entity \EasyCorp\Bundle\EasyAdminBundle\Dto\EntityDto #}
44
{# NOTE: the rel="noopener" attr is needed to avoid performance and security issues
55
(see https://web.dev/external-anchors-use-rel-noopener/) #}
6-
{% if ea().crud.currentAction == 'detail' %}
6+
{% if field.customOptions.get('isUnsafe') %}
7+
<span class="text-muted">{{ field.value }}</span>
8+
{% elseif ea().crud.currentAction == 'detail' %}
79
<a target="_blank" rel="noopener" href="{{ field.value }}">{{ field.value }}</a>
810
{% else %}
911
<a target="_blank" rel="noopener" href="{{ field.value }}">{{ field.formattedValue }}</a>

‎tests/Unit/Field/UrlFieldTest.php‎

Lines changed: 240 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33
namespace EasyCorp\Bundle\EasyAdminBundle\Tests\Unit\Field;
44

55
use EasyCorp\Bundle\EasyAdminBundle\Config\Action;
6+
use EasyCorp\Bundle\EasyAdminBundle\Config\Crud;
67
use EasyCorp\Bundle\EasyAdminBundle\Field\Configurator\UrlConfigurator;
78
use EasyCorp\Bundle\EasyAdminBundle\Field\UrlField;
9+
use Symfony\Component\Validator\Constraints\Url;
810

911
class UrlFieldTest extends AbstractFieldTest
1012
{
@@ -59,6 +61,244 @@ public function testFormattedValuesOnIndexAction(string $url, string $expectedRe
5961
self::assertSame($expectedRenderedUrl, $fieldDto->getFormattedValue());
6062
}
6163

64+
/**
65+
* @testWith ["javascript:alert(1)"]
66+
* ["JavaScript:alert(1)"]
67+
* ["JaVaScRiPt:alert(1)"]
68+
* ["data:text/html,<script>alert(1)</script>"]
69+
* ["vbscript:msgbox(1)"]
70+
* ["file:///etc/passwd"]
71+
* [" javascript:alert(1)"]
72+
* ["\tjavascript:alert(1)"]
73+
* ["\njavascript:alert(1)"]
74+
* ["\u0000javascript:alert(1)"]
75+
*/
76+
public function testDangerousSchemesAreMarkedUnsafe(string $url): void
77+
{
78+
$this->initializeConfigurator();
79+
80+
$field = UrlField::new('foo')->setValue($url);
81+
$fieldDto = $this->configure($field);
82+
83+
self::assertTrue($fieldDto->getCustomOption(UrlField::OPTION_IS_UNSAFE));
84+
}
85+
86+
/**
87+
* @testWith ["http://example.com"]
88+
* ["https://example.com"]
89+
* ["ftp://example.com"]
90+
* ["ftps://example.com"]
91+
* ["ssh://user@example.com"]
92+
* ["sftp://user@example.com"]
93+
* ["git://example.com/repo.git"]
94+
* ["mailto:user@example.com"]
95+
* ["webcal://example.com/cal.ics"]
96+
* ["/relative/path"]
97+
* ["relative/path"]
98+
* ["//example.com/protocol-relative"]
99+
* [""]
100+
*/
101+
public function testSafeSchemesAreNotMarkedUnsafe(string $url): void
102+
{
103+
$this->initializeConfigurator();
104+
105+
$field = UrlField::new('foo')->setValue($url);
106+
$fieldDto = $this->configure($field);
107+
108+
self::assertFalse($fieldDto->getCustomOption(UrlField::OPTION_IS_UNSAFE));
109+
}
110+
111+
public function testTemplateRendersDangerousUrlAsSpanInsteadOfLink(): void
112+
{
113+
$this->initializeConfigurator();
114+
115+
$field = UrlField::new('foo')->setValue('javascript:alert(1)');
116+
$fieldDto = $this->configure($field);
117+
118+
$html = $this->renderFieldTemplate($fieldDto, $this->entityDto, $this->adminContext);
119+
120+
// the dangerous scheme must not be rendered inside an anchor's href attribute
121+
self::assertStringNotContainsString('<a ', $html);
122+
self::assertStringNotContainsString('href=', $html);
123+
self::assertStringContainsString('<span', $html);
124+
self::assertStringContainsString('javascript:alert(1)', $html);
125+
}
126+
127+
public function testTemplateEscapesDangerousUrlContainingHtml(): void
128+
{
129+
$this->initializeConfigurator();
130+
131+
$field = UrlField::new('foo')->setValue('data:text/html,<script>alert(1)</script>');
132+
$fieldDto = $this->configure($field);
133+
134+
$html = $this->renderFieldTemplate($fieldDto, $this->entityDto, $this->adminContext);
135+
136+
self::assertStringNotContainsString('<script>', $html);
137+
self::assertStringContainsString('&lt;script&gt;', $html);
138+
}
139+
140+
public function testTemplateRendersSafeUrlAsLink(): void
141+
{
142+
$this->initializeConfigurator();
143+
144+
$field = UrlField::new('foo')->setValue('https://example.com');
145+
$fieldDto = $this->configure($field);
146+
147+
$html = $this->renderFieldTemplate($fieldDto, $this->entityDto, $this->adminContext);
148+
149+
self::assertStringContainsString('<a ', $html);
150+
self::assertStringContainsString('href="https://example.com"', $html);
151+
self::assertStringContainsString('target="_blank"', $html);
152+
self::assertStringContainsString('rel="noopener"', $html);
153+
}
154+
155+
public function testTemplateRendersDangerousUrlAsSpanOnDetailAction(): void
156+
{
157+
$this->initializeConfigurator();
158+
159+
$field = UrlField::new('foo')->setValue('javascript:alert(1)');
160+
$fieldDto = $this->configure($field, pageName: Crud::PAGE_DETAIL, actionName: Action::DETAIL);
161+
162+
$html = $this->renderFieldTemplate($fieldDto, $this->entityDto, $this->adminContext);
163+
164+
self::assertStringNotContainsString('<a ', $html);
165+
self::assertStringContainsString('<span', $html);
166+
}
167+
168+
public function testAllowedProtocolsOptionDefaultsToNull(): void
169+
{
170+
$this->initializeConfigurator();
171+
172+
$field = UrlField::new('foo');
173+
$fieldDto = $this->configure($field, actionName: Action::EDIT);
174+
175+
self::assertNull($fieldDto->getCustomOption(UrlField::OPTION_ALLOWED_PROTOCOLS));
176+
self::assertNull($fieldDto->getFormTypeOption('constraints'));
177+
}
178+
179+
public function testAllowedProtocolsAddsUrlConstraint(): void
180+
{
181+
$this->initializeConfigurator();
182+
183+
$field = UrlField::new('foo')->allowedProtocols(['http', 'https']);
184+
$fieldDto = $this->configure($field, actionName: Action::EDIT);
185+
186+
self::assertSame(['http', 'https'], $fieldDto->getCustomOption(UrlField::OPTION_ALLOWED_PROTOCOLS));
187+
188+
$constraints = $fieldDto->getFormTypeOption('constraints');
189+
self::assertIsArray($constraints);
190+
self::assertCount(1, $constraints);
191+
self::assertInstanceOf(Url::class, $constraints[0]);
192+
self::assertSame(['http', 'https'], $constraints[0]->protocols);
193+
}
194+
195+
public function testAllowedProtocolsAppendsToExistingConstraints(): void
196+
{
197+
$this->initializeConfigurator();
198+
199+
$existingConstraint = new Url();
200+
$field = UrlField::new('foo')
201+
->setFormTypeOption('constraints', [$existingConstraint])
202+
->allowedProtocols(['ftp', 'sftp']);
203+
$fieldDto = $this->configure($field, actionName: Action::EDIT);
204+
205+
$constraints = $fieldDto->getFormTypeOption('constraints');
206+
self::assertIsArray($constraints);
207+
self::assertCount(2, $constraints);
208+
self::assertSame($existingConstraint, $constraints[0]);
209+
self::assertInstanceOf(Url::class, $constraints[1]);
210+
self::assertSame(['ftp', 'sftp'], $constraints[1]->protocols);
211+
}
212+
213+
public function testAllowedProtocolsReturnsSelfForFluentInterface(): void
214+
{
215+
$field = UrlField::new('foo');
216+
217+
self::assertSame($field, $field->allowedProtocols(['http', 'https']));
218+
}
219+
220+
/**
221+
* @testWith [["http"]]
222+
* [["https"]]
223+
* [["ftp", "ftps"]]
224+
* [["ssh", "sftp", "git"]]
225+
* [["http", "https", "ftp", "mailto", "webcal"]]
226+
*/
227+
public function testAllowedProtocolsAcceptsDifferentProtocolSets(array $protocols): void
228+
{
229+
$this->initializeConfigurator();
230+
231+
$field = UrlField::new('foo')->allowedProtocols($protocols);
232+
$fieldDto = $this->configure($field, actionName: Action::EDIT);
233+
234+
self::assertSame($protocols, $fieldDto->getCustomOption(UrlField::OPTION_ALLOWED_PROTOCOLS));
235+
236+
$constraints = $fieldDto->getFormTypeOption('constraints');
237+
self::assertCount(1, $constraints);
238+
self::assertInstanceOf(Url::class, $constraints[0]);
239+
self::assertSame($protocols, $constraints[0]->protocols);
240+
}
241+
242+
public function testAllowedProtocolsWithEmptyArrayStillAddsConstraint(): void
243+
{
244+
$this->initializeConfigurator();
245+
246+
$field = UrlField::new('foo')->allowedProtocols([]);
247+
$fieldDto = $this->configure($field, actionName: Action::EDIT);
248+
249+
self::assertSame([], $fieldDto->getCustomOption(UrlField::OPTION_ALLOWED_PROTOCOLS));
250+
251+
$constraints = $fieldDto->getFormTypeOption('constraints');
252+
self::assertCount(1, $constraints);
253+
self::assertInstanceOf(Url::class, $constraints[0]);
254+
self::assertSame([], $constraints[0]->protocols);
255+
}
256+
257+
public function testAllowedProtocolsLastCallWins(): void
258+
{
259+
$this->initializeConfigurator();
260+
261+
$field = UrlField::new('foo')
262+
->allowedProtocols(['http', 'https'])
263+
->allowedProtocols(['ftp']);
264+
$fieldDto = $this->configure($field, actionName: Action::EDIT);
265+
266+
self::assertSame(['ftp'], $fieldDto->getCustomOption(UrlField::OPTION_ALLOWED_PROTOCOLS));
267+
268+
$constraints = $fieldDto->getFormTypeOption('constraints');
269+
self::assertCount(1, $constraints);
270+
self::assertSame(['ftp'], $constraints[0]->protocols);
271+
}
272+
273+
public function testAllowedProtocolsIsIndependentFromDefaultProtocol(): void
274+
{
275+
$this->initializeConfigurator();
276+
277+
$field = UrlField::new('foo')
278+
->setDefaultProtocol('https')
279+
->allowedProtocols(['http', 'https']);
280+
$fieldDto = $this->configure($field, actionName: Action::EDIT);
281+
282+
self::assertSame('https', $fieldDto->getCustomOption(UrlField::OPTION_DEFAULT_PROTOCOL));
283+
self::assertSame('https', $fieldDto->getFormTypeOption('default_protocol'));
284+
self::assertSame(['http', 'https'], $fieldDto->getCustomOption(UrlField::OPTION_ALLOWED_PROTOCOLS));
285+
self::assertCount(1, $fieldDto->getFormTypeOption('constraints'));
286+
}
287+
288+
public function testNoConstraintIsAddedWhenAllowedProtocolsIsNotCalled(): void
289+
{
290+
$this->initializeConfigurator();
291+
292+
$existingConstraint = new Url();
293+
$field = UrlField::new('foo')
294+
->setFormTypeOption('constraints', [$existingConstraint]);
295+
$fieldDto = $this->configure($field, actionName: Action::EDIT);
296+
297+
$constraints = $fieldDto->getFormTypeOption('constraints');
298+
self::assertCount(1, $constraints);
299+
self::assertSame($existingConstraint, $constraints[0]);
300+
}
301+
62302
private function initializeConfigurator(): void
63303
{
64304
self::bootKernel();

0 commit comments

Comments
 (0)