Skip to content

Fix HTML injection in comments and meeting's description#8511

Merged
andreslucena merged 4 commits intodecidim:developfrom
i-need-another-coffee:fix/html-injection
Dec 10, 2021
Merged

Fix HTML injection in comments and meeting's description#8511
andreslucena merged 4 commits intodecidim:developfrom
i-need-another-coffee:fix/html-injection

Conversation

@roxanaopr
Copy link
Copy Markdown
Contributor

@roxanaopr roxanaopr commented Nov 9, 2021

🎩 What? Why?

Comment's body and meeting's description are not escaping the html structure.
Trying to submit a comment on a proposal/meeting that contains HTML code will result in that code being treated as part of the page. This was observed as well on the description field from meetings.

📌 Related Issues

Testing

Add the following comment on a comment or meeting

Some description
<form action="/action_page.php">
<label for="fname">First name:</label>
<input type="text" id="fname" name="fname"><br><br>
<label for="lname">Last name:</label>
<input type="text" id="lname" name="lname"><br><br>
<input type="submit" value="Submit">
</form>

The html structure will be render in the page

📋 Checklist

🚨 Please review the guidelines for contributing to this repository.

  • CONSIDER adding a unit test if your PR resolves an issue.
  • ✔️ DO check open PR's to avoid duplicates.
  • ✔️ DO keep pull requests small so they can be easily reviewed.
  • ✔️ DO build locally before pushing.
  • ✔️ DO make sure tests pass.
  • ✔️ DO make sure any new changes are documented in docs/.
  • ✔️ DO add and modify seeds if necessary.
  • ✔️ DO add CHANGELOG upgrade notes if required.
  • ✔️ DO add to GraphQL API if there are new public fields.
  • ✔️ DO add link to MetaDecidim if it's a new feature.
  • AVOID breaking the continuous integration build.
  • AVOID making significant changes to the overall architecture.

📷 Screenshots

Screenshot from 2021-10-21 10-30-53

♥️ Thank you!

@andreslucena andreslucena changed the title HTML injection in comments and meeting's description #8510 HTML injection in comments and meeting's description Nov 10, 2021
@roxanaopr roxanaopr force-pushed the fix/html-injection branch 2 times, most recently from 94c9206 to d7b2685 Compare November 10, 2021 17:23
@roxanaopr roxanaopr marked this pull request as ready for review November 11, 2021 13:15
@andreslucena
Copy link
Copy Markdown
Member

Good job @roxanaopr!!

I tried it locally and it works as expected.

Related to this PR, as we've talked last week, I also added the explanation and screenshots on how the comments feature works.

@andreslucena andreslucena linked an issue Nov 15, 2021 that may be closed by this pull request
Copy link
Copy Markdown
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

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

Great job on moving the proposal sanitization logic and reusing that for meetings! 👏🏽
I have a few suggestions, could you please check them?
Also, there are some merge conflicts to solve.
Thanks for this PR!!

@andreslucena andreslucena changed the title HTML injection in comments and meeting's description Fix HTML injection in comments and meeting's description Nov 29, 2021
@andreslucena andreslucena added the type: fix PRs that implement a fix for a bug label Nov 29, 2021
@andreslucena andreslucena merged commit cd29028 into decidim:develop Dec 10, 2021
@andreslucena
Copy link
Copy Markdown
Member

Thanks for the PR @roxanaopr
As this is a fix, could you please make a backport to release/0.25-stable 🙏🏽 ? Thanks

entantoencuanto added a commit to PopulateTools/decidim that referenced this pull request Dec 10, 2021
* fix/meetings_form_embed_type_visibility:
  Fix tests by adding missing doubled attributes
  Include value in validation conditional
  Allow participants to set iframe access level of meetings
  Fix embed type visibility in participants form
  Remove blank option in meetings embed type select
  Fix avatar thumbnail in participants' profile (decidim#8577)
  Fix HTML injection in comments and meeting's description (decidim#8511)
  Add search, filters and sorting to admin panel budget projects (decidim#8592)
  Add cache key separator to cache_hash (decidim#8559)
  Move social login buttons to the top of the login modal (decidim#8574)
  Fix the meeting copy functionality (decidim#8430)
  Temporarily ignore CSS validation issue in CI (decidim#8597)
  Fix security instructions (decidim#8587)
alecslupu pushed a commit to i-need-another-coffee/decidim that referenced this pull request Dec 14, 2021
alecslupu pushed a commit to i-need-another-coffee/decidim that referenced this pull request Dec 23, 2021
@alecslupu alecslupu added this to the 0.26.0 milestone Jul 14, 2023
@alecslupu alecslupu deleted the fix/html-injection branch October 31, 2024 05:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HTML injection in comments and meeting's description Raw text formatting on meeting body

3 participants