Fix XSS vulnerability in title attribute #847
Conversation
…he title attribute.
|
Any chance of getting this merged in? An outstanding security issue that's approaching 5 years old doesn't bode well for any upstream consumers of this project. |
|
There are more details in #846, in short: from a backwards compatibility point of view this can be considered a feature 🤷♂️ |
|
The ticket does describe a workaround. So even though the defaults aren't safe, you can still get the library to behave in a safe manner by escaping the user controlled input yourself. |
|
@Addono that doesn't help for other projects that use this, like the Drupal colorbox module. Anyone know what happened to the maintainers? |
|
Also, it's not a feature request when said Drupal module is now considered unsupported because of this and THOUSANDS of sites are using that. I really consider that a cop-out, I'm afraid. :( It's a royal pain in the ass to fix something like this when you're dealing with a dependency chain like that. For the record, I'd be much happier if this stuff could be done without JS altogether, like some sort of extension of HTML5. I hate having to depend on JS for this crap, because it's always brittle. |
|
@seanr I don't think this library is responsible for how the Drupal module uses it. The Drupal module can and should prevent malicious text from being used in these labels, that's all. Questions about maintainership of the Drupal module will likely be answered better/faster in the Drupal module queue and are a bit off-topic over here. |
|
My point is there's a known bug in the underlying library. It should be fixed here. |
|
It's not a bug, though. From #846 it's explained that allowing HTML is intentional and whatever uses this should do the work of appropriate sanitizing. It's the responsibility of higher-level code that knows "was the text entered by an admin or an untrusted user?" to do appropriate filtering/sanitizing. |
|
I think the rationality in the issue discussion makes sense. Perhaps it would be best to close this PR? |
|
@LukeLeber Agreed. But thank you all for the input! Colorbox continues to exist mainly for legacy users, so a backward compatibility change (even if released as a new version) has the potential to cause more trouble than it's worth. |
No description provided.