- Issue created by @neptuneDG
- π©πͺGermany Anybody Porta Westfalica
Thanks for the report @neptuneDG. While we should never use
|raw
for security reasons, I think there are already some related or even duplicate issues which discuss this problem.I think it's something that needs to be changed and solved in the root logics of this module, so that escaping works like in all other cases. I'd be super happy if someone could take the time to check more in deep, where things go wrong and why. It's since day 1, I guess.
- π―π΅Japan neptuneDG
If you think that |raw is unsafe in this situation, then you already have a vulnerability. In the very same twig, you call the following line:
<article id="{{ id }}" {{attributes.addClass(classes)}}>
The output of attributes.addClass(classes) is marked as safe by twig and returns all attributes without escapes. Because the attributes I have marked as |raw are also inside that, I am not adding an additional vulnerability with this change.
At the moment in your current implementation, your data attributes are not escaped, yet your user-visible tags are escaped.
- π―π΅Japan neptuneDG
Do you think your code is related to this change in drupal 9? https://www.drupal.org/project/drupal/issues/2297711 β
- π©πͺGermany Anybody Porta Westfalica
Honestly it's not my code, but from the original maintainer of this module. We should try to not use
|raw
where ever possible and it's no option for me here, until I'm 100% sure, there's no better way. And I'm quite sure, there are better ways.So I agree with the issue, but not with the solution. Too busy currently to look into it deeper, but of course I'll review MR's when ever possible. Thanks for your report and work on this.
- π΅π±Poland kewin.nammert
I've noticed that moving labels from nested #attributes to the main array fixes the problem, but I'm not sure that's a good solution. If it is I can do the MR. Somehow passing the values as Markup which shouldn't be escaped doesn't have an effect either. Any ideas why?