Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX] Improved argument validation in TagBuilder (#937) #969

Merged
merged 1 commit into from
Aug 14, 2024
Merged

Conversation

s2b
Copy link
Contributor

@s2b s2b commented Aug 14, 2024

The current approach to sanitize HTML tag attributes by
TagBuilder can be improved to only allow certain characters.
As TagBuilder is a low-level API to create arbitrary HTML tags,
the goal is not to prevent any JavaScript from being executed
because this might be a desired behavior in some use cases.

We would like to prevent the following:

$unsafeInput = "onclick='alert(1)'";
$tagBuilder->addAttribute($unsafeInput, 'some value');

While still allowing the following:

$tagBuilder->addAttribute('onclick', 'doSomething()');

Thus, this patch applies a strict allow-list to argument names
and throws an exception if any other characters are used.
The characters are limited to ASCII characters except for
select characters that are problematic in HTML attributes,
such as single and double quotes, equal signs, less/greater than,
forward slashes, ampersants and white space.

Developers are advised to be very careful when using user input
as attribute names as this change cannot prevent all accidential
and thus potentially malicious JavaScript execution.

Thanks to @GatekeeperBuster for making us aware of this.

The current approach to sanitize HTML tag attributes by
TagBuilder can be improved to only allow certain characters.
As TagBuilder is a low-level API to create arbitrary HTML tags,
the goal is not to prevent any JavaScript from being executed
because this might be a desired behavior in some use cases.

We would like to prevent the following:

```php
$unsafeInput = "onclick='alert(1)'";
$tagBuilder->addAttribute($unsafeInput, 'some value');
```

While still allowing the following:

```php
$tagBuilder->addAttribute('onclick', 'doSomething()');
```

Thus, this patch applies a strict allow-list to argument names
and throws an exception if any other characters are used.
The characters are limited to ASCII characters except for
select characters that are problematic in HTML attributes,
such as single and double quotes, equal signs, less/greater than,
forward slashes, ampersants and white space.

Developers are advised to be very careful when using user input
as attribute names as this change cannot prevent all accidential
and thus potentially malicious JavaScript execution.

Thanks to @GatekeeperBuster for making us aware of this.
@s2b s2b merged commit 42f389f into 2.15 Aug 14, 2024
10 checks passed
@s2b s2b deleted the backport branch August 14, 2024 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant