CVE-2021-43827: Improper Handling of Exceptional Conditions (CWE-755)
value: discourse-footnote is a library providing footnotes for posts in Discourse. ### Impact When posting an inline footnote wrapped in `<a>` tags (e.g. `<a>^[footnote]</a>`, the resulting rendered HTML would include a nested `<a>`, which is stripped by Nokogiri because it is not valid. This then caused a javascript error on topic pages because we were looking for an `<a>` element inside the footnote reference span and getting its ID, and because it did not exist we got a null reference error in javascript. Users are advised to update to version 0.2. As a workaround editing offending posts from the rails console or the database console for self-hosters, or disabling the plugin in the admin panel can mitigate this issue.
CWE description: The product does not handle or incorrectly handles an exceptional condition.
CVSS v3 base score: 4.3
Published: 2021-12-14T23:15Z
Repository: https://github.com/discourse/discourse-footnote
Language: JavaScript
Files changed: assets/javascripts/initializers/inline-footnotes.js, assets/vendor/javascripts/markdown-it-footnote.js, plugin.rb, spec/pretty_text_spec.rb
Commit date: 2021-12-14 15:35:11 +1000
Commit message: SECURITY: Fix malicious footnote causing clientside errors (#27)

Some malformed/malicious footnote markup could cause
clientside errors.
Diff: diff --git a/assets/javascripts/initializers/inline-footnotes.js b/assets/javascripts/initializers/inline-footnotes.js
index c7327b2..5bdd1ea 100644
--- a/assets/javascripts/initializers/inline-footnotes.js
+++ b/assets/javascripts/initializers/inline-footnotes.js
@@ -6,18 +6,24 @@ let inlineFootnotePopper;
 
 function applyInlineFootnotes(elem) {
   const footnoteRefs = elem.querySelectorAll("sup.footnote-ref");
 
   footnoteRefs.forEach((footnoteRef) => {
+    const refLink = footnoteRef.querySelector("a");
+    if (!refLink) {
+      return;
+    }
+
     const expandableFootnote = document.createElement("a");
     expandableFootnote.classList.add("expand-footnote");
     expandableFootnote.innerHTML = iconHTML("ellipsis-h");
     expandableFootnote.href = "";
     expandableFootnote.role = "button";
-    expandableFootnote.dataset.footnoteId = footnoteRef
-      .querySelector("a")
-      .id.replace("footnote-ref-", "");
+    expandableFootnote.dataset.footnoteId = refLink.id.replace(
+      "footnote-ref-",
+      ""
+    );
 
     footnoteRef.after(expandableFootnote);
   });
 
   if (footnoteRefs.length) {
diff --git a/assets/vendor/javascripts/markdown-it-footnote.js b/assets/vendor/javascripts/markdown-it-footnote.js
index 122eabc..0ec8054 100644
--- a/assets/vendor/javascripts/markdown-it-footnote.js
+++ b/assets/vendor/javascripts/markdown-it-footnote.js
@@ -210,10 +210,19 @@ module.exports = function footnote_plugin(md) {
         state.md,
         state.env,
         tokens = []
       );
 
+      // Having a rendered footnote inside a link creates a nested link, which
+      // is not valid HTML, so close the parent tag first before proceeding.
+      const previousToken = state.tokens[state.tokens.length - 1];
+      if (previousToken?.content.includes("<a")) {
+        const linkCloseToken = state.push("html_inline", "", 0);
+        linkCloseToken.content = "</a>";
+        linkCloseToken.block = false;
+      }
+
       token      = state.push('footnote_ref', '', 0);
       token.meta = { id: footnoteId };
 
       state.env.footnotes.list[footnoteId] = {
         content: state.src.slice(labelStart, labelEnd),
diff --git a/plugin.rb b/plugin.rb
index eed568c..541b359 100644
--- a/plugin.rb
+++ b/plugin.rb
@@ -1,10 +1,10 @@
 # frozen_string_literal: true
 # transpile_js: true
 # name: discourse-footnote
 # about: Adds markdown.it footnote support to Discourse
-# version: 0.1
+# version: 0.2
 # authors: Sam Saffron, Vitaly Puzrin
 # url: https://github.com/discourse/discourse-footnote
 
 enabled_site_setting :enable_markdown_footnotes
 
diff --git a/spec/pretty_text_spec.rb b/spec/pretty_text_spec.rb
index fea49cd..2b7de8e 100644
--- a/spec/pretty_text_spec.rb
+++ b/spec/pretty_text_spec.rb
@@ -85,6 +85,33 @@ describe PrettyText do
 
     expect(post.cooked.strip).to eq(html.strip)
 
   end
 
+  it "supports inline footnotes wrapped in <a> elements by ending the elements early" do
+    raw = <<~MD
+      I have a point, see footnote. <a>^[the point]</a>
+
+      <a>^[footnote]</a>
+    MD
+
+    post = create_post(raw: raw)
+    post.reload
+
+    html = <<~HTML
+      <p>I have a point, see footnote. <a></a><sup class="footnote-ref"><a href="#footnote-#{post.id}-1" id="footnote-ref-#{post.id}-1">[1]</a></sup></p>
+      <p><a></a><sup class="footnote-ref"><a href="#footnote-#{post.id}-2" id="footnote-ref-#{post.id}-2">[2]</a></sup></p>
+      <hr class="footnotes-sep">
+
+      <ol class="footnotes-list">
+      <li id="footnote-#{post.id}-1" class="footnote-item">
+      <p>the point <a href="#footnote-ref-#{post.id}-1" class="footnote-backref">↩︎</a></p>
+      </li>
+      <li id="footnote-#{post.id}-2" class="footnote-item">
+      <p>footnote <a href="#footnote-ref-#{post.id}-2" class="footnote-backref">↩︎</a></p>
+      </li>
+      </ol>
+    HTML
+
+    expect(post.cooked.strip).to eq(html.strip)
+  end
 end

Vulnerable code:     expandableFootnote.dataset.footnoteId = footnoteRef
      .querySelector("a")
      .id.replace("footnote-ref-", "");
# version: 0.1
Fixed code:     const refLink = footnoteRef.querySelector("a");
    if (!refLink) {
      return;
    }

    expandableFootnote.dataset.footnoteId = refLink.id.replace(
      "footnote-ref-",
      ""
    );
      // Having a rendered footnote inside a link creates a nested link, which
      // is not valid HTML, so close the parent tag first before proceeding.
      const previousToken = state.tokens[state.tokens.length - 1];
      if (previousToken?.content.includes("<a")) {
        const linkCloseToken = state.push("html_inline", "", 0);
        linkCloseToken.content = "</a>";
        linkCloseToken.block = false;
      }

# version: 0.2
  it "supports inline footnotes wrapped in <a> elements by ending the elements early" do
    raw = <<~MD
      I have a point, see footnote. <a>^[the point]</a>

      <a>^[footnote]</a>
    MD

    post = create_post(raw: raw)
    post.reload

    html = <<~HTML
      <p>I have a point, see footnote. <a></a><sup class="footnote-ref"><a href="#footnote-#{post.id}-1" id="footnote-ref-#{post.id}-1">[1]</a></sup></p>
      <p><a></a><sup class="footnote-ref"><a href="#footnote-#{post.id}-2" id="footnote-ref-#{post.id}-2">[2]</a></sup></p>
      <hr class="footnotes-sep">

      <ol class="footnotes-list">
      <li id="footnote-#{post.id}-1" class="footnote-item">
      <p>the point <a href="#footnote-ref-#{post.id}-1" class="footnote-backref">↩︎</a></p>
      </li>
      <li id="footnote-#{post.id}-2" class="footnote-item">
      <p>footnote <a href="#footnote-ref-#{post.id}-2" class="footnote-backref">↩︎</a></p>
      </li>
      </ol>
    HTML

    expect(post.cooked.strip).to eq(html.strip)
  end