CVE-2023-51447: Insufficient Information (NVD-CWE-noinfo)
value: Decidim is a participatory democracy framework. Starting in version 0.27.0 and prior to versions 0.27.5 and 0.28.0, the dynamic file upload feature is subject to potential cross-site scripting attacks in case the attacker manages to modify the file names of the records being uploaded to the server. This appears in sections where the user controls the file upload dialogs themselves and has the technical knowledge to change the file names through the dynamic upload endpoint. Therefore I believe it would require the attacker to control the whole session of the particular user but in any case, this needs to be fixed. Successful exploit of this vulnerability would require the user to have successfully uploaded a file blob to the server with a malicious file name and then have the possibility to direct the other user to the edit page of the record where the attachment is attached. The users are able to craft the direct upload requests themselves controlling the file name that gets stored to the database. The attacker is able to change the filename e.g. to `<svg onload=alert('XSS')>` if they know how to craft these requests themselves. And then enter the returned blob ID to the form inputs manually by modifying the edit page source. Versions 0.27.5 and 0.28.0 contain a patch for this issue. As a workaround, disable dynamic uploads for the instance, e.g. from proposals.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-02-20T18:15Z
Repository: https://github.com/decidim/decidim
Language: JavaScript
Files changed: decidim-core/app/cells/decidim/upload_modal_cell.rb, decidim-core/app/packs/src/decidim/direct_uploads/redesigned_upload_modal.js, decidim-core/app/packs/src/decidim/direct_uploads/upload_modal.js, decidim-core/app/packs/src/decidim/utilities/text.js, decidim-core/spec/cells/decidim/upload_modal_cell_spec.rb, decidim-proposals/spec/system/edit_proposal_spec.rb
Commit date: 2023-09-26 12:16:21 +0300
Commit message: Fix issues with the file uploader input display (#11612)


Diff: diff --git a/decidim-core/app/cells/decidim/upload_modal_cell.rb b/decidim-core/app/cells/decidim/upload_modal_cell.rb
index 05efd5d4b..0056b908f 100644
--- a/decidim-core/app/cells/decidim/upload_modal_cell.rb
+++ b/decidim-core/app/cells/decidim/upload_modal_cell.rb
@@ -157,19 +157,19 @@ module Decidim
 
       decidim_html_escape(decidim_sanitize(translated_attribute(attachment.title)))
     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)
       return attachment.filename.to_s if attachment.is_a? ActiveStorage::Blob
       return blob(attachment).filename.to_s if blob(attachment).present?
diff --git a/decidim-core/app/packs/src/decidim/direct_uploads/redesigned_upload_modal.js b/decidim-core/app/packs/src/decidim/direct_uploads/redesigned_upload_modal.js
index 202ab2c86..14353eaef 100644
--- a/decidim-core/app/packs/src/decidim/direct_uploads/redesigned_upload_modal.js
+++ b/decidim-core/app/packs/src/decidim/direct_uploads/redesigned_upload_modal.js
@@ -1,8 +1,9 @@
 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",
   ERROR: "error"
 }
@@ -164,32 +165,32 @@ 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>
     `
 
     const titleTemplate = `
       <img src="" alt="${file.name}" />
       <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>
     `
 
     let state = STATUS.VALIDATED
@@ -210,11 +211,11 @@ export default class UploadModal {
     // eslint-disable-next-line no-ternary
     const attachmentId = opts.attachmentId
       ? `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>
         </div>
         <progress max="100" value="${opts.value || 0}"></progress>
@@ -246,11 +247,11 @@ export default class UploadModal {
 
     this.updateDropZone();
   }
 
   setProgressBar(name, value) {
-    this.uploadItems.querySelector(`[data-filename="${name}"] progress`).value = value
+    this.uploadItems.querySelector(`[data-filename="${escapeQuotes(name)}"] progress`).value = value
   }
 
   updateAddAttachmentsButton() {
     if (this.uploadItems.children.length === 0) {
       this.button.innerHTML = this.modalTitle.dataset.addlabel;
diff --git a/decidim-core/app/packs/src/decidim/direct_uploads/upload_modal.js b/decidim-core/app/packs/src/decidim/direct_uploads/upload_modal.js
index 9b8004d36..4b5be4ef9 100644
--- a/decidim-core/app/packs/src/decidim/direct_uploads/upload_modal.js
+++ b/decidim-core/app/packs/src/decidim/direct_uploads/upload_modal.js
@@ -1,7 +1,8 @@
 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.
 export default class UploadModal {
   constructor(button, options = {}) {
@@ -68,11 +69,11 @@ export default class UploadModal {
       } else {
         const ordinalNumber = this.getOrdinalNumber();
 
         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);
 
         const hiddenBlobField = createHiddenInput(null, null, blob.signed_id);
@@ -82,14 +83,14 @@ export default class UploadModal {
           hiddenBlobField.name = `${this.options.resourceName}[${this.options.addAttribute}]`;
         }
 
         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) {
           this.cleanTrashCan();
         }
@@ -125,11 +126,11 @@ 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");
     const thirdRow = document.createElement("div");
     firstRow.classList.add("row", "upload-item-first-row");
@@ -142,11 +143,11 @@ export default class UploadModal {
       fileNameSpanClasses.push("small-4", "medium-5");
     } else {
       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");
     if (state) {
       if (state === "validated") {
@@ -177,18 +178,17 @@ export default class UploadModal {
     const removeButton = document.createElement("button");
     removeButton.classList.add("columns", "small-3", "medium-2", "remove-upload-item");
     removeButton.innerHTML = `&times; ${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);
     thirdRow.appendChild(errorList);
 
diff --git a/decidim-core/app/packs/src/decidim/utilities/text.js b/decidim-core/app/packs/src/decidim/utilities/text.js
new file mode 100644
index 000000000..2f3a712db
--- /dev/null
+++ b/decidim-core/app/packs/src/decidim/utilities/text.js
@@ -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, "&quot;");
+}
diff --git a/decidim-core/spec/cells/decidim/upload_modal_cell_spec.rb b/decidim-core/spec/cells/decidim/upload_modal_cell_spec.rb
index e0ab451de..5f71c7f5f 100644
--- a/decidim-core/spec/cells/decidim/upload_modal_cell_spec.rb
+++ b/decidim-core/spec/cells/decidim/upload_modal_cell_spec.rb
@@ -145,10 +145,26 @@ describe Decidim::UploadModalCell, type: :cell do
 
         details = subject.find(".attachment-details")
         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
     let(:filename1) { "Exampledocument.pdf" }
     let(:filename2) { "city.jpeg" }
diff --git a/decidim-proposals/spec/system/edit_proposal_spec.rb b/decidim-proposals/spec/system/edit_proposal_spec.rb
index 19b5187b4..4984ffdc0 100644
--- a/decidim-proposals/spec/system/edit_proposal_spec.rb
+++ b/decidim-proposals/spec/system/edit_proposal_spec.rb
@@ -108,10 +108,54 @@ describe "Edit proposals", type: :system do
             expect(Decidim::Attachment.count).to eq(2)
             expect(translated(Decidim::Attachment.find_by(attached_to_id: proposal.id, content_type: "image/jpeg").title)).to eq(attachment_image_title)
             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
         it "can add many images many times" do
           skip "REDESIGN_PENDING - Flaky test: upload modal fails on GitHub with multiple fileshttps://github.com/decidim/decidim/issues/10961"

Vulnerable code:       filename = file_name_for(attachment)
      return filename if filename.length <= max_length
      name.truncate(max_length, omission: "...#{name.last((max_length / 2) - 3)}#{File.extname(filename)}")
      determine_filename(attachment)
      <span>${truncateFilename(file.name)}</span>
        <span>${truncateFilename(file.name)}</span>
          <span>${truncateFilename(file.name)}</span>
          <input class="sm" type="text" value="${opts.title || truncateFilename(file.name)}" />
      <li ${attachmentId} data-filename="${file.name}" data-state="${state}">
    this.uploadItems.querySelector(`[data-filename="${name}"] progress`).value = value
        attachmentDetails.dataset.filename = file.name;
          titleAndFileNameSpan.innerHTML = `${title} (${file.name})`;
          titleAndFileNameSpan.innerHTML = file.name;
    wrapper.setAttribute("data-filename", fileName);
    fileNameSpan.innerHTML = truncateFilename(fileName);
      const item = this.uploadItems.querySelector(`[data-filename='${fileName}']`);
      this.trashCan.append(item);
    titleAndFileNameSpan.innerHTML = `${title} (${truncateFilename(fileName)})`;
Fixed code:       filename = determine_filename(attachment)
      return decidim_html_escape(filename).html_safe if filename.length <= max_length
      decidim_html_escape(name.truncate(max_length, omission: "...#{name.last((max_length / 2) - 3)}#{File.extname(filename)}")).html_safe
      decidim_html_escape(determine_filename(attachment)).html_safe
import { escapeHtml, escapeQuotes } from "src/decidim/utilities/text";
      <span>${escapeHtml(truncateFilename(file.name))}</span>
        <span>${escapeHtml(truncateFilename(file.name))}</span>
          <span>${escapeHtml(truncateFilename(file.name))}</span>
          <input class="sm" type="text" value="${escapeQuotes(opts.title || truncateFilename(file.name))}" />
      <li ${attachmentId} data-filename="${escapeQuotes(file.name)}" data-state="${state}">
    this.uploadItems.querySelector(`[data-filename="${escapeQuotes(name)}"] progress`).value = value
import { escapeHtml } from "src/decidim/utilities/text";
        attachmentDetails.dataset.filename = escapeHtml(file.name);
          titleAndFileNameSpan.innerHTML = escapeHtml(`${title} (${file.name})`);
          titleAndFileNameSpan.innerHTML = escapeHtml(file.name);
    wrapper.setAttribute("data-filename", escapeHtml(fileName));
    fileNameSpan.innerHTML = escapeHtml(truncateFilename(fileName));
      this.trashCan.append(wrapper);
    titleAndFileNameSpan.innerHTML = escapeHtml(`${title} (${truncateFilename(fileName)})`);
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, "&quot;");
}

    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

        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