Skip to content
This repository has been archived by the owner on Oct 24, 2023. It is now read-only.

Commit

Permalink
SECURITY: Fix malicious footnote causing clientside errors (#27)
Browse files Browse the repository at this point in the history
Some malformed/malicious footnote markup could cause
clientside errors.
  • Loading branch information
martin-brennan committed Dec 14, 2021
1 parent 11d1a66 commit 796617e
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 4 deletions.
12 changes: 9 additions & 3 deletions assets/javascripts/initializers/inline-footnotes.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,20 @@ 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);
});
Expand Down
9 changes: 9 additions & 0 deletions assets/vendor/javascripts/markdown-it-footnote.js
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,15 @@ module.exports = function footnote_plugin(md) {
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 };

Expand Down
2 changes: 1 addition & 1 deletion plugin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# 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

Expand Down
27 changes: 27 additions & 0 deletions spec/pretty_text_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,31 @@

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

0 comments on commit 796617e

Please sign in to comment.