Skip to content

Conversation

@Stef-Rousset
Copy link
Contributor

🎩 Description

This PR removes escaping from categorie's names in FO.

Testing

As an admin, go to a process > Categories and update the name of a category, adding special characters (like '&"<>).
As a user, go to the proposals of the process and see that the category's name you have updated is displayed with its special characters not escaped.

📌 Related Issues

Link your PR to an issue

Tasks

  • Add specs

📷 Screenshots

BO
Capture d’écran 2025-09-30 à 11 30 26

FO
Capture d’écran 2025-09-30 à 11 28 59

Copy link
Contributor

@alecslupu alecslupu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Stef-Rousset head's up!


categories_values = sorted_main_categories.flat_map do |category|
sorted_descendant_categories = category.descendants.includes(:subcategories).sort_by do |subcategory|
[subcategory.weight, translated_attribute(subcategory.name)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use here a "decidim_sanitize_translated", to avoid XSS ?

Suggested change
[subcategory.weight, translated_attribute(subcategory.name)]
[subcategory.weight, decidim_sanitize_translated(subcategory.name)]

end

subcategories = sorted_descendant_categories.flat_map do |subcategory|
Decidim::CheckBoxesTreeHelper::TreePoint.new(subcategory.id.to_s, translated_attribute(subcategory.name))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not a ?

Suggested change
Decidim::CheckBoxesTreeHelper::TreePoint.new(subcategory.id.to_s, translated_attribute(subcategory.name))
Decidim::CheckBoxesTreeHelper::TreePoint.new(subcategory.id.to_s, decidim_sanitize_translated(subcategory.name))

end

Decidim::CheckBoxesTreeHelper::TreeNode.new(
Decidim::CheckBoxesTreeHelper::TreePoint.new(category.id.to_s, translated_attribute(category.name)),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Decidim::CheckBoxesTreeHelper::TreePoint.new(category.id.to_s, translated_attribute(category.name)),
Decidim::CheckBoxesTreeHelper::TreePoint.new(category.id.to_s, decidim_sanitize_translated(category.name)),

@Stef-Rousset
Copy link
Contributor Author

Hello @alecslupu , thanks for your review ! I totally understand your concern about the security. I have a little question about this case, and I will appreciate your expertise on it. I tested (in the decidim-app) the categories without escape in front, and the <script >alert('XSS')</script> is not executed in the BO nor in the FO, even in the FO if I filter with it on the proposals index for instance, or I create a new proposal.

Also, the scopes, which are as categories created in the BO, are not sanitized in the BO nor in the FO, and the script <script >alert('XSS scope')</script> is executed in the BO when you arrive on the process show page, and also in the FO when you create a proposal.
<img widt
Capture d’écran 2025-10-06 à 11 07 03
Capture d’écran 2025-10-06 à 11 07 51

I would really appreciate your analysis on this topic, thanks and have a nice day !

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.

3 participants