Skip to content

Don't allow v-html anymore as long as content is not explicitely trusted

Stefan C requested to merge reduce-xss-risk into master

Closes #1840

We have a problem that XSS vulnerabilities to our site are introduced again and again.

It would be great to get rid of them. The most common cause are v-html tags in vue.

The Idea is, to use them only if there is no other way and then to be sure, that the content is at least sanitized, even if this is not 100% secure.

What does this MR do?

  • Adds error to linter, when v-html is included.
  • Remove v-html tag almost everywhere (some style things might break and needs to be fixed, but i think this is better then having the security issues

TODOs:

  • Check where issues in displaying content are.
  • Add missing sanitizing of admin content
  • Check if new commits have introduces more v-html tags before merging (done until ab9458a8)

How confident are you it won't break things if deployed?

Places where HTML content was used for formatting might break

unrelated for follow-up issue

  • LinkField.vue and LinkContainer.vue are probably unused

Links to related issues

How to test

Add <script>alert('{{ id }}')</script> everywhere (Every DB Field directly in the DB, every variable...) and check if it is just displayed and not executed, when there is still v-html used

Further, each change should be checked if displaying issues are there and try to find a way without v-html or if really not possible, sanitize content.


Layout checked or fixed:

  • client/src/components/Banners/Broadcast/BroadcastField.vue
  • client/src/components/Banners/Errors/ErrorField.vue
  • client/src/components/Banners/Informations/InformationField.vue
  • client/src/components/Banners/Intro/IntroField.vue
  • client/src/components/Banners/Quiz/QuizField.vue
  • client/src/components/Basket/RequestForm.vue
  • client/src/components/Container/activity/ActivityOverview.vue
  • client/src/components/Container/activity/ActivityPost.vue
  • client/src/components/Container/basket/BasketField.vue
  • client/src/components/Container/event/EventField.vue
  • client/src/components/Container/group/GroupField.vue
  • client/src/components/Container/link/LinkField.vue
  • client/src/components/Container/pickup/PickupField.vue
  • client/src/components/Container/poll/PollField.vue
  • client/src/components/Container/region/RegionField.vue
  • client/src/components/Container/store/JumpingStoreContainer.vue
  • client/src/components/Container/store/ManagingStoreContainer.vue
  • client/src/components/Container/store/StoreContainer.vue
  • client/src/components/Container/store/StoreField.vue
  • client/src/components/Mailbox/MailboxSingleEmailView.vue
  • client/src/components/Modals/Donation/DonationModal.vue
  • client/src/components/Navigation/Baskets/NavBasketsEntry.vue
  • client/src/components/Navigation/Conversations/NavConversationsEntry.vue
  • client/src/components/Navigation/Groups/NavGroupsEntry.vue
  • client/src/components/Navigation/Notifications/NavNotificationsEntry.vue
  • client/src/components/Navigation/Regions/NavRegionsEntry.vue
  • client/src/components/Navigation/Stores/NavStores.vue
  • client/src/components/Navigation/_NavItems/NavDropdown.vue
  • client/src/components/Navigation/_NavItems/NavLink.vue
  • client/src/components/Stores/Pickup/Pickup.vue
  • client/src/components/Stores/StoreLogEntryMessage.vue
  • client/src/components/map/LeafletLocationSearch.vue
  • client/src/views/pages/Index/Index.vue
  • client/src/views/partials/Footer/Footer.vue
  • client/src/views/partials/Modals/JoinRegionModal.vue
  • client/src/views/partials/Modals/StyleGuideModal.vue
  • client/src/views/partials/Navigation/States/MainNav/LoggedIn.vue
  • client/src/views/partials/Navigation/States/MainNav/LoggedOut.vue
  • src/Modules/Blog/components/BlogPost.vue
  • src/Modules/Blog/components/BlogPostListItem.vue
  • src/Modules/Map/components/StoreBubble.vue
  • src/Modules/Profile/components/ProfileMenu.vue
  • src/Modules/Register/components/RegisterBirthdate.vue
  • src/Modules/Report/components/ReportList.vue
  • src/Modules/Settings/components/NameInput.vue

Checklist

  • added a test, or explain why one is not needed/possible...
  • no unrelated changes
  • asked someone for a code review
  • set a "for:" label to indicate who will be affected by this change
  • added to the next milestone (see https://gitlab.com/foodsharing-dev/foodsharing/-/milestones, unless it has a "for:Dev" label)
  • added an entry to CHANGELOG.md
  • added a short text in the release notes to /release-notes/YYYY-MM.md
  • Once your MR has been merged, you are responsible to create a testing issue in the Beta Testing forum: https://foodsharing.de/region?bid=734&sub=forum. Please change the MRs label to "state:Beta testing".
    • Consider writing a detailed description in German.
    • Describe in a few sentences, what should be tested from a user perspective.
    • Also mention different settings (e.g. different browsers, roles, ...) how this change can be tested.
    • Be aware, that also non technical people should understand.
Edited by Stefan C

Merge request reports