torrust / torrust-index-gui

This repository serves as the frontend for the Torrust Index project.
https://torrust.com
Other
32 stars 16 forks source link

Review lint warning: `warning 'v-html' directive can lead to XSS attack vue/no-v-html` #105

Closed josecelano closed 1 year ago

josecelano commented 1 year ago

The component components/Markdown.vue uses the v-html directive that can lead to XSS attack.

I have added an ESLint comment to hide the warning here:

<template>
  <!-- eslint-disable vue/no-v-html -->
  <div class="prose" v-html="sanitizedDescription" />
</template>

Because we are sanitizing the description with the sanitizeDescription function:

async function sanitizeDescription () {
  // Get the original not sanitized markdown string.
  const description = markdown(props.source);

  // Replace the img src's with a random id and return a map
  // of these ids mapped to the original url.
  const [filteredDescriptionWithImageIds, imageIdUrlMap] = filterDescriptionImagesWithRandomIds(description);

  // Get the image data using the backend's image proxy.
  const imageIdDataUrlMap = await getImageDataUrlsFromUrls(imageIdUrlMap);

  // Replace the img id's with the proxied sources.
  sanitizedDescription.value = replaceDescriptionImageIdsWithDataUrls(filteredDescriptionWithImageIds, imageIdDataUrlMap);
}

But I wonder if we are still vulnerable to other XSS attacks. We should double-check it. We could use a library like RisXSS.

Links

da2ce7 commented 1 year ago

We can consider https://github.com/cure53/DOMPurify also.

josecelano commented 1 year ago

I confirm you can inject whatever. If you add this torrent description:

<p>Click the button to display an alert box.</p>

<button onclick="alert('Hello! I am an alert box!')">Try it</button>

You can show a link (Try it) for a popup alert.

image

josecelano commented 1 year ago

I'm using this text to test the sanitize function:

Harmful script (the button show not show the alert):

<p>Click the button to display an alert box.</p>

<button onclick="alert('Hello! I am an alert box!')">Try it</button>

Valid PNG image in markdown format (it should show the image using the proxy)

![Torrust Logo](https://raw.githubusercontent.com/torrust/torrust-index-backend/develop/docs/media/torrust_logo.png)

Another valid (JPG) image in markdown format (it should show the image using the proxy)

![Mandelbrot Set](https://upload.wikimedia.org/wikipedia/commons/2/21/Mandel_zoom_00_mandelbrot_set.jpg)

Valid image (PNG) in html format (it should show the image using the proxy):

<img src="https://raw.githubusercontent.com/torrust/torrust-index-backend/develop/docs/media/torrust_logo.png">

Invalid image (TIF) in html format (it should remove the image):

<img src="https://commons.wikimedia.org/wiki/Category:TIFF_files#/media/File:Arkansas_Constitution_of_1836,_page_1.tif">

Invalid html link (it should remove the href attribute value):

<a href="https://commons.wikimedia.org/wiki/Category:TIFF_files#/media/File:Arkansas_Constitution_of_1836,_page_1.tif">Arkansas Constitution of 1836. Page 1</a>

<embed type="video/webm"
       src="/media/cc0-videos/flower.mp4"
       width="250"
       height="200">

I think we should remove all HTML tags containing internal sources, not only images and links.