Decidim vulnerable to cross-site scripting (XSS) in the dynamic file uploads

@@ -159,15 +159,15 @@ def title_for(attachment)
     end
 
     def truncated_file_name_for(attachment, max_length = 31)
-      filename = file_name_for(attachment)
-      return filename if filename.length <= max_length
+      filename = determine_filename(attachment)
+      return decidim_html_escape(filename).html_safe if filename.length <= max_length
 
       name = File.basename(filename, File.extname(filename))
-      name.truncate(max_length, omission: "...#{name.last((max_length / 2) - 3)}#{File.extname(filename)}")
+      decidim_html_escape(name.truncate(max_length, omission: "...#{name.last((max_length / 2) - 3)}#{File.extname(filename)}")).html_safe
     end
 
     def file_name_for(attachment)
-      determine_filename(attachment)
+      decidim_html_escape(determine_filename(attachment)).html_safe
     end
 
     def determine_filename(attachment)

@@ -1,6 +1,7 @@
 import { Uploader } from "src/decidim/direct_uploads/redesigned_uploader";
 import icon from "src/decidim/redesigned_icon";
 import { truncateFilename } from "src/decidim/direct_uploads/upload_utility";
+import { escapeHtml, escapeQuotes } from "src/decidim/utilities/text";
 
 const STATUS = {
   VALIDATED: "validated",
@@ -166,13 +167,13 @@ export default class UploadModal {
   createUploadItem(file, errors, opts = {}) {
     const okTemplate = `
       <img src="" alt="${file.name}" />
-      <span>${truncateFilename(file.name)}</span>
+      <span>${escapeHtml(truncateFilename(file.name))}</span>
     `
 
     const errorTemplate = `
       <div>${icon("error-warning-line")}</div>
       <div>
-        <span>${truncateFilename(file.name)}</span>
+        <span>${escapeHtml(truncateFilename(file.name))}</span>
         <span>${this.locales.validation_error}</span>
         <ul>${errors.map((error) => `<li>${error}</li>`).join("\n")}</ul>
       </div>
@@ -183,11 +184,11 @@ export default class UploadModal {
       <div>
         <div>
           <label>${this.locales.filename}</label>
-          <span>${truncateFilename(file.name)}</span>
+          <span>${escapeHtml(truncateFilename(file.name))}</span>
         </div>
         <div>
           <label>${this.locales.title}</label>
-          <input class="sm" type="text" value="${opts.title || truncateFilename(file.name)}" />
+          <input class="sm" type="text" value="${escapeQuotes(opts.title || truncateFilename(file.name))}" />
         </div>
       </div>
     `
@@ -212,7 +213,7 @@ export default class UploadModal {
       ? `data-attachment-id="${opts.attachmentId}"`
       : ""
     const fullTemplate = `
-      <li ${attachmentId} data-filename="${file.name}" data-state="${state}">
+      <li ${attachmentId} data-filename="${escapeQuotes(file.name)}" data-state="${state}">
         <div data-template="${template}">
           ${content.trim()}
           <button>${this.locales.remove}</button>
@@ -248,7 +249,7 @@ export default class UploadModal {
   }
 
   setProgressBar(name, value) {
-    this.uploadItems.querySelector(`[data-filename="${name}"] progress`).value = value
+    this.uploadItems.querySelector(`[data-filename="${escapeQuotes(name)}"] progress`).value = value
   }
 
   updateAddAttachmentsButton() {

@@ -1,5 +1,6 @@
 import { Uploader } from "src/decidim/direct_uploads/uploader";
 import { truncateFilename, checkTitles, createHiddenInput } from "src/decidim/direct_uploads/upload_utility";
+import { escapeHtml } from "src/decidim/utilities/text";
 
 // This class handles logic inside upload modal, but since modal is not inside the form
 // logic here moves "upload items" / hidden inputs to form.
@@ -70,7 +71,7 @@ export default class UploadModal {
 
         const attachmentDetails = document.createElement("div");
         attachmentDetails.classList.add("attachment-details");
-        attachmentDetails.dataset.filename = file.name;
+        attachmentDetails.dataset.filename = escapeHtml(file.name);
         const titleAndFileNameSpan = document.createElement("span");
         titleAndFileNameSpan.style.display = "none";
         attachmentDetails.appendChild(titleAndFileNameSpan);
@@ -84,10 +85,10 @@ export default class UploadModal {
 
         if (this.options.titled) {
           const hiddenTitleField = createHiddenInput("hidden-title", `${this.options.resourceName}[${this.options.addAttribute}][${ordinalNumber}][title]`, title);
-          titleAndFileNameSpan.innerHTML = `${title} (${file.name})`;
+          titleAndFileNameSpan.innerHTML = escapeHtml(`${title} (${file.name})`);
           attachmentDetails.appendChild(hiddenTitleField);
         } else {
-          titleAndFileNameSpan.innerHTML = file.name;
+          titleAndFileNameSpan.innerHTML = escapeHtml(file.name);
         }
 
         if (!this.options.multiple) {
@@ -127,7 +128,7 @@ export default class UploadModal {
   createUploadItem(fileName, title, state) {
     const wrapper = document.createElement("div");
     wrapper.classList.add("upload-item");
-    wrapper.setAttribute("data-filename", fileName);
+    wrapper.setAttribute("data-filename", escapeHtml(fileName));
 
     const firstRow = document.createElement("div");
     const secondRow = document.createElement("div");
@@ -144,7 +145,7 @@ export default class UploadModal {
       fileNameSpanClasses.push("small-12");
     }
     fileNameSpan.classList.add(...fileNameSpanClasses);
-    fileNameSpan.innerHTML = truncateFilename(fileName);
+    fileNameSpan.innerHTML = escapeHtml(truncateFilename(fileName));
 
     const progressBar = document.createElement("div");
     progressBar.classList.add("progress-bar");
@@ -179,14 +180,13 @@ export default class UploadModal {
     removeButton.innerHTML = `× ${this.locales.remove}`;
     removeButton.addEventListener(("click"), (event) => {
       event.preventDefault();
-      const item = this.uploadItems.querySelector(`[data-filename='${fileName}']`);
-      this.trashCan.append(item);
+      this.trashCan.append(wrapper);
       this.updateDropZone();
     })
 
     const titleAndFileNameSpan = document.createElement("span");
     titleAndFileNameSpan.classList.add("columns", "small-5", "title-and-filename-span");
-    titleAndFileNameSpan.innerHTML = `${title} (${truncateFilename(fileName)})`;
+    titleAndFileNameSpan.innerHTML = escapeHtml(`${title} (${truncateFilename(fileName)})`);
 
     firstRow.appendChild(fileNameSpan);
     secondRow.appendChild(progressBarWrapper);

@@ -0,0 +1,17 @@
+export const escapeHtml = (text) => {
+  if (!text) {
+    return "";
+  }
+
+  const el = document.createElement("div");
+  el.appendChild(document.createTextNode(text));
+  return el.innerHTML;
+}
+
+export const escapeQuotes = (text) => {
+  if (!text) {
+    return "";
+  }
+
+  return text.replace(/"/g, """);
+}

@@ -147,6 +147,22 @@ def model_name
         expect(details).to have_content("#{attachments[0].title["en"]} (#{filename})")
       end
     end
+
+    context "when there is rich content in the filename" do
+      let(:blob) { ActiveStorage::Blob.find_signed(attachments.first) }
+
+      before do
+        blob.update!(filename: "<svg onload=alert('ALERT')>.pdf")
+      end
+
+      it "escapes the truncated filename" do
+        expect(my_cell.send(:truncated_file_name_for, attachments.first)).to eq("&lt;svg onload=alert(&#39;ALERT&#39;)&gt;.pdf")
+      end
+
+      it "escapes the filename" do
+        expect(my_cell.send(:file_name_for, attachments.first)).to eq("&lt;svg onload=alert(&#39;ALERT&#39;)&gt;.pdf")
+      end
+    end
   end
 
   context "when multiple attachments are present" do

@@ -110,6 +110,50 @@
             expect(translated(Decidim::Attachment.find_by(attached_to_id: proposal.id, content_type: "application/pdf").title)).to eq(attachment_file_title)
           end
         end
+
+        context "with problematic file titles" do
+          let!(:photo) { create(:attachment, :with_image, weight: 0, attached_to: proposal) }
+          let!(:document) { create(:attachment, :with_pdf, weight: 1, attached_to: proposal) }
+
+          before do
+            document.update!(title: { en: "<svg onload=alert('ALERT')>.pdf" })
+            photo.update!(title: { en: "<svg onload=alert('ALERT')>.jpg" })
+          end
+
+          it "displays them correctly on the edit form" do
+            # With problematic code, should raise Selenium::WebDriver::Error::UnexpectedAlertOpenError
+            click_link "Edit proposal"
+            expect(page).to have_content("Required fields are marked with an asterisk")
+            click_button("Edit documents")
+            within "[data-dialog]" do
+              click_button("Next")
+            end
+            click_button("Send")
+            expect(page).to have_content("Proposal successfully updated.")
+          end
+        end
+
+        context "with problematic file names" do
+          let!(:photo) { create(:attachment, :with_image, weight: 0, attached_to: proposal) }
+          let!(:document) { create(:attachment, :with_pdf, weight: 1, attached_to: proposal) }
+
+          before do
+            document.file.blob.update!(filename: "<svg onload=alert('ALERT')>.pdf")
+            photo.file.blob.update!(filename: "<svg onload=alert('ALERT')>.jpg")
+          end
+
+          it "displays them correctly on the edit form" do
+            # With problematic code, should raise Selenium::WebDriver::Error::UnexpectedAlertOpenError
+            click_link "Edit proposal"
+            expect(page).to have_content("Required fields are marked with an asterisk")
+            click_button("Edit documents")
+            within "[data-dialog]" do
+              click_button("Next")
+            end
+            click_button("Send")
+            expect(page).to have_content("Proposal successfully updated.")
+          end
+        end
       end
 
       context "with multiple images", :slow do