Merge pull request commonmark#229 from github/fix-footnotes-nested-linkrefs-autolinker

Fix footnotes: nested, confused for link refs & mangled by the autolinker

Author: phillmv

src/blocks.c

@@ -468,7 +468,6 @@ static void process_footnotes(cmark_parser *parser) {
   while ((ev_type = cmark_iter_next(iter)) != CMARK_EVENT_DONE) {
     cur = cmark_iter_get_node(iter);
     if (ev_type == CMARK_EVENT_EXIT && cur->type == CMARK_NODE_FOOTNOTE_DEFINITION) {
-      cmark_node_unlink(cur);
       cmark_footnote_create(map, cur);
     }
   }
@@ -515,13 +514,16 @@ static void process_footnotes(cmark_parser *parser) {
     qsort(map->sorted, map->size, sizeof(cmark_map_entry *), sort_footnote_by_ix);
     for (unsigned int i = 0; i < map->size; ++i) {
       cmark_footnote *footnote = (cmark_footnote *)map->sorted[i];
-      if (!footnote->ix)
+      if (!footnote->ix) {
+        cmark_node_unlink(footnote->node);
         continue;
+      }
       cmark_node_append_child(parser->root, footnote->node);
       footnote->node = NULL;
     }
   }
 
+  cmark_unlink_footnotes_map(map);
   cmark_map_free(map);
 }
 

src/footnotes.c

@@ -38,3 +38,26 @@ void cmark_footnote_create(cmark_map *map, cmark_node *node) {
 cmark_map *cmark_footnote_map_new(cmark_mem *mem) {
   return cmark_map_new(mem, footnote_free);
 }
+
+// Before calling `cmark_map_free` on a map with `cmark_footnotes`, first
+// unlink all of the footnote nodes before freeing their memory.
+//
+// Sometimes, two (unused) footnote nodes can end up referencing each other,
+// which as they get freed up by calling `cmark_map_free` -> `footnote_free` ->
+// etc, can lead to a use-after-free error.
+//
+// Better to `unlink` every footnote node first, setting their next, prev, and
+// parent pointers to NULL, and only then walk thru & free them up.
+void cmark_unlink_footnotes_map(cmark_map *map) {
+  cmark_map_entry *ref;
+  cmark_map_entry *next;
+
+  ref = map->refs;
+  while(ref) {
+    next = ref->next;
+    if (((cmark_footnote *)ref)->node) {
+      cmark_node_unlink(((cmark_footnote *)ref)->node);
+    }
+    ref = next;
+  }
+}

src/footnotes.h

@@ -18,6 +18,8 @@ typedef struct cmark_footnote cmark_footnote;
 void cmark_footnote_create(cmark_map *map, cmark_node *node);
 cmark_map *cmark_footnote_map_new(cmark_mem *mem);
 
+void cmark_unlink_footnotes_map(cmark_map *map);
+
 #ifdef __cplusplus
 }
 #endif

src/inlines.c

@@ -1137,19 +1137,77 @@ static cmark_node *handle_close_bracket(cmark_parser *parser, subject *subj) {
   // What if we're a footnote link?
   if (parser->options & CMARK_OPT_FOOTNOTES &&
       opener->inl_text->next &&
-      opener->inl_text->next->type == CMARK_NODE_TEXT &&
-      !opener->inl_text->next->next) {
+      opener->inl_text->next->type == CMARK_NODE_TEXT) {
+
     cmark_chunk *literal = &opener->inl_text->next->as.literal;
-    if (literal->len > 1 && literal->data[0] == '^') {
-      inl = make_simple(subj->mem, CMARK_NODE_FOOTNOTE_REFERENCE);
-      inl->as.literal = cmark_chunk_dup(literal, 1, literal->len - 1);
-      inl->start_line = inl->end_line = subj->line;
-      inl->start_column = opener->inl_text->start_column;
-      inl->end_column = subj->pos + subj->column_offset + subj->block_offset;
-      cmark_node_insert_before(opener->inl_text, inl);
-      cmark_node_free(opener->inl_text->next);
-      cmark_node_free(opener->inl_text);
+
+    // look back to the opening '[', and skip ahead to the next character
+    // if we're looking at a '[^' sequence, and there is other text or nodes
+    // after the ^, let's call it a footnote reference.
+    if ((literal->len > 0 && literal->data[0] == '^') && (literal->len > 1 || opener->inl_text->next->next)) {
+
+      // Before we got this far, the `handle_close_bracket` function may have
+      // advanced the current state beyond our footnote's actual closing
+      // bracket, ie if it went looking for a `link_label`.
+      // Let's just rewind the subject's position:
+      subj->pos = initial_pos;
+
+      cmark_node *fnref = make_simple(subj->mem, CMARK_NODE_FOOTNOTE_REFERENCE);
+
+      // the start and end of the footnote ref is the opening and closing brace
+      // i.e. the subject's current position, and the opener's start_column
+      int fnref_end_column = subj->pos + subj->column_offset + subj->block_offset;
+      int fnref_start_column = opener->inl_text->start_column;
+
+      // any given node delineates a substring of the line being processed,
+      // with the remainder of the line being pointed to thru its 'literal'
+      // struct member.
+      // here, we copy the literal's pointer, moving it past the '^' character
+      // for a length equal to the size of footnote reference text.
+      // i.e. end_col minus start_col, minus the [ and the ^ characters
+      //
+      // this copies the footnote reference string, even if between the
+      // `opener` and the subject's current position there are other nodes
+      //
+      // (first, check for underflows)
+      if ((fnref_start_column + 2) <= fnref_end_column) {
+        fnref->as.literal = cmark_chunk_dup(literal, 1, (fnref_end_column - fnref_start_column) - 2);
+      } else {
+        fnref->as.literal = cmark_chunk_dup(literal, 1, 0);
+      }
+
+      fnref->start_line = fnref->end_line = subj->line;
+      fnref->start_column = fnref_start_column;
+      fnref->end_column = fnref_end_column;
+
+      // we then replace the opener with this new fnref node, the net effect
+      // being replacing the opening '[' text node with a `^footnote-ref]` node.
+      cmark_node_insert_before(opener->inl_text, fnref);
+
       process_emphasis(parser, subj, opener->previous_delimiter);
+      // sometimes, the footnote reference text gets parsed into multiple nodes
+      // i.e. '[^example]' parsed into '[', '^exam', 'ple]'.
+      // this happens for ex with the autolink extension. when the autolinker
+      // finds the 'w' character, it will split the text into multiple nodes
+      // in hopes of being able to match a 'www.' substring.
+      //
+      // because this function is called one character at a time via the
+      // `parse_inlines` function, and the current subj->pos is pointing at the
+      // closing ] brace, and because we copy all the text between the [ ]
+      // braces, we should be able to safely ignore and delete any nodes after
+      // the opener->inl_text->next.
+      //
+      // therefore, here we walk thru the list and free them all up
+      cmark_node *next_node;
+      cmark_node *current_node = opener->inl_text->next;
+      while(current_node) {
+        next_node = current_node->next;
+        cmark_node_free(current_node);
+        current_node = next_node;
+      }
+
+      cmark_node_free(opener->inl_text);
+
       pop_bracket(subj);
       return NULL;
     }

test/regression.txt

@@ -269,3 +269,99 @@ Pull request #128 - Buffer overread in tables extension
 <p>|
 -|</p>
 ````````````````````````````````
+
+Footnotes may be nested inside other footnotes.
+
+```````````````````````````````` example footnotes
+This is some text. It has a citation.[^citation]
+
+[^another-citation]: My second citation.
+
+[^citation]: This is a long winded parapgraph that also has another citation.[^another-citation]
+.
+<p>This is some text. It has a citation.<sup class="footnote-ref"><a href="#fn1" id="fnref1">1</a></sup></p>
+<section class="footnotes">
+<ol>
+<li id="fn1">
+<p>This is a long winded parapgraph that also has another citation.<sup class="footnote-ref"><a href="#fn2" id="fnref2">2</a></sup> <a href="#fnref1" class="footnote-backref">↩</a></p>
+</li>
+<li id="fn2">
+<p>My second citation. <a href="#fnref2" class="footnote-backref">↩</a></p>
+</li>
+</ol>
+</section>
+````````````````````````````````
+
+Footnotes are similar to, but should not be confused with, link references
+
+```````````````````````````````` example footnotes
+This is some text. It has two footnotes references, side-by-side without any spaces,[^footnote1][^footnote2] which are definitely not link references.
+
+[^footnote1]: Hello.
+
+[^footnote2]: Goodbye.
+.
+<p>This is some text. It has two footnotes references, side-by-side without any spaces,<sup class="footnote-ref"><a href="#fn1" id="fnref1">1</a></sup><sup class="footnote-ref"><a href="#fn2" id="fnref2">2</a></sup> which are definitely not link references.</p>
+<section class="footnotes">
+<ol>
+<li id="fn1">
+<p>Hello. <a href="#fnref1" class="footnote-backref">↩</a></p>
+</li>
+<li id="fn2">
+<p>Goodbye. <a href="#fnref2" class="footnote-backref">↩</a></p>
+</li>
+</ol>
+</section>
+````````````````````````````````
+
+Footnotes may begin with or have a 'w' or a '_' in their reference label.
+
+```````````````````````````````` example footnotes autolink
+This is some text. Sometimes the autolinker splits up text into multiple nodes, hoping it will find a hyperlink, so this text has a footnote whose reference label begins with a `w`.[^widely-cited]
+
+It has another footnote that contains many different characters (the autolinker was also breaking on `_`).[^sphinx-of-black-quartz_judge-my-vow-0123456789]
+
+[^sphinx-of-black-quartz_judge-my-vow-0123456789]: so does this.
+
+[^widely-cited]: this renders properly.
+.
+<p>This is some text. Sometimes the autolinker splits up text into multiple nodes, hoping it will find a hyperlink, so this text has a footnote whose reference label begins with a <code>w</code>.<sup class="footnote-ref"><a href="#fn1" id="fnref1">1</a></sup></p>
+<p>It has another footnote that contains many different characters (the autolinker was also breaking on <code>_</code>).<sup class="footnote-ref"><a href="#fn2" id="fnref2">2</a></sup></p>
+<section class="footnotes">
+<ol>
+<li id="fn1">
+<p>this renders properly. <a href="#fnref1" class="footnote-backref">↩</a></p>
+</li>
+<li id="fn2">
+<p>so does this. <a href="#fnref2" class="footnote-backref">↩</a></p>
+</li>
+</ol>
+</section>
+````````````````````````````````
+
+Footnotes interacting with strikethrough should not lead to a use-after-free
+
+```````````````````````````````` example footnotes autolink strikethrough table
+|Tot.....[^_a_]|
+.
+<p>|Tot.....[^_a_]|</p>
+````````````````````````````````
+
+Footnotes interacting with strikethrough should not lead to a use-after-free pt2
+
+```````````````````````````````` example footnotes autolink strikethrough table
+[^~~is~~1]
+.
+<p>[^~~is~~1]</p>
+````````````````````````````````
+
+Adjacent unused footnotes definitions should not lead to a use after free
+
+```````````````````````````````` example footnotes autolink strikethrough table
+Hello world
+
+
+[^a]:[^b]:
+.
+<p>Hello world</p>
+````````````````````````````````