[BUGFIX] Improved argument validation in TagBuilder

[s2b]

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.

This will be downported to Fluid 2.15.

Thanks to @GatekeeperBuster for making us aware of this.

Resolves: #936

[liayn]

Thanks for the work done here. My take on this: We cannot cover all cases properly. Attributes are the responsibility of the integrator. The warning about being careful about attributes created from user-input should be places somewhere really well visible.

Code-wise I would not try to validate anything, otherwise more reports will pop-up like "nice, but you do not cover XYZ". That's not going to help us. I'd stick with the motto "Do it right or not at all." in this case.

[s2b]

@liayn Thank you for your feedback. I agree that it isn't really possible to cover all cases here. However, if we would follow "Do it right or not at all.", we should also remove htmlspecialchars() here because it covers even less cases.

Based on the feedback of the TYPO3 security channel, I'd argue that we should merge this imperfect improvement. The validation is better than before and a clear error is given if something is obviously wrong.