From 74631f8610d1067570d107e7c701f3e602e6c32e Mon Sep 17 00:00:00 2001 From: Phill MV Date: Tue, 3 Aug 2021 12:15:37 -0400 Subject: [PATCH 01/36] Copied over the actions CI workflows that @jgm added in commonmark/cmark. --- .github/workflows/ci.yml | 102 +++++++++++++++++++++++++++++++++++++ .github/workflows/fuzz.yml | 23 +++++++++ 2 files changed, 125 insertions(+) create mode 100644 .github/workflows/ci.yml create mode 100644 .github/workflows/fuzz.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 000000000..8a0d2c1ca --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,102 @@ +name: CI tests + +on: [push, pull_request, workflow_dispatch] + +jobs: + + linter: + + runs-on: ubuntu-latest + + steps: + + - uses: actions/checkout@v1 + - name: Install clang-tidy + run: | + sudo apt-get install -y clang-tidy-9 + sudo update-alternatives --install /usr/bin/clang-tidy clang-tidy /usr/bin/clang-tidy-9 100 + - name: lint with clang-tidy + run: | + make lint + env: + CC: clang + CXX: clang++ + + linux: + + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + cmake_opts: + - '-DCMARK_SHARED=ON' + - '' + compiler: + - c: 'clang' + cpp: 'clang++' + - c: 'gcc' + cpp: 'g++' + env: + CMAKE_OPTIONS: ${{ matrix.cmake_opts }} + CC: ${{ matrix.compiler.c }} + CXX: ${{ matrix.compiler.cpp }} + + steps: + - uses: actions/checkout@v1 + - name: Install valgrind + run: | + sudo apt install -y valgrind + - name: Build and test + run: | + make + make test + make leakcheck + + macos: + + runs-on: macOS-latest + strategy: + fail-fast: false + matrix: + cmake_opts: + - '-DCMARK_SHARED=ON' + - '' + compiler: + - c: 'clang' + cpp: 'clang++' + - c: 'gcc' + cpp: 'g++' + env: + CMAKE_OPTIONS: ${{ matrix.cmake_opts }} + CC: ${{ matrix.compiler.c }} + CXX: ${{ matrix.compiler.cpp }} + + steps: + - uses: actions/checkout@v1 + - name: Build and test + env: + CMAKE_OPTIONS: -DCMARK_SHARED=OFF + run: | + make + make test + + windows: + + runs-on: windows-latest + strategy: + fail-fast: false + matrix: + cmake_opts: + - '-DCMARK_SHARED=ON' + - '' + env: + CMAKE_OPTIONS: ${{ matrix.cmake_opts }} + + steps: + - uses: actions/checkout@v1 + - uses: ilammy/msvc-dev-cmd@v1 + - name: Build and test + run: | + chcp 65001 + nmake.exe /nologo /f Makefile.nmake test + shell: cmd diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml new file mode 100644 index 000000000..c918fd81c --- /dev/null +++ b/.github/workflows/fuzz.yml @@ -0,0 +1,23 @@ +name: CIFuzz +on: [pull_request, workflow_dispatch] +jobs: + Fuzzing: + runs-on: ubuntu-latest + steps: + - name: Build Fuzzers + uses: google/oss-fuzz/infra/cifuzz/actions/build_fuzzers@master + with: + oss-fuzz-project-name: 'cmark' + dry-run: false + - name: Run Fuzzers + uses: google/oss-fuzz/infra/cifuzz/actions/run_fuzzers@master + with: + oss-fuzz-project-name: 'cmark' + fuzz-seconds: 600 + dry-run: false + - name: Upload Crash + uses: actions/upload-artifact@v1 + if: failure() + with: + name: artifacts + path: ./out/artifacts From 7346f8dedc8ba382180a4b87caa6df49769bc4e4 Mon Sep 17 00:00:00 2001 From: Phill MV Date: Tue, 3 Aug 2021 12:19:00 -0400 Subject: [PATCH 02/36] Turns out we don't have a lint task in our Makefile (yet). --- .github/workflows/ci.yml | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8a0d2c1ca..7b7dd1d67 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3,25 +3,6 @@ name: CI tests on: [push, pull_request, workflow_dispatch] jobs: - - linter: - - runs-on: ubuntu-latest - - steps: - - - uses: actions/checkout@v1 - - name: Install clang-tidy - run: | - sudo apt-get install -y clang-tidy-9 - sudo update-alternatives --install /usr/bin/clang-tidy clang-tidy /usr/bin/clang-tidy-9 100 - - name: lint with clang-tidy - run: | - make lint - env: - CC: clang - CXX: clang++ - linux: runs-on: ubuntu-latest From 2b2ffafb675fb9d36072a84f826f5cc658371a12 Mon Sep 17 00:00:00 2001 From: Phill MV Date: Tue, 3 Aug 2021 12:22:35 -0400 Subject: [PATCH 03/36] Let's drop the fuzzer for now, looks like we have to set something up with oss-fuzz first. --- .github/workflows/fuzz.yml | 23 ----------------------- 1 file changed, 23 deletions(-) delete mode 100644 .github/workflows/fuzz.yml diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml deleted file mode 100644 index c918fd81c..000000000 --- a/.github/workflows/fuzz.yml +++ /dev/null @@ -1,23 +0,0 @@ -name: CIFuzz -on: [pull_request, workflow_dispatch] -jobs: - Fuzzing: - runs-on: ubuntu-latest - steps: - - name: Build Fuzzers - uses: google/oss-fuzz/infra/cifuzz/actions/build_fuzzers@master - with: - oss-fuzz-project-name: 'cmark' - dry-run: false - - name: Run Fuzzers - uses: google/oss-fuzz/infra/cifuzz/actions/run_fuzzers@master - with: - oss-fuzz-project-name: 'cmark' - fuzz-seconds: 600 - dry-run: false - - name: Upload Crash - uses: actions/upload-artifact@v1 - if: failure() - with: - name: artifacts - path: ./out/artifacts From 4e0a81fff706e76a1f2ad84a7d3e033f97ac424f Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Mon, 9 Mar 2020 13:54:22 -0700 Subject: [PATCH 04/36] Make normalize test compatible with python 3.8 Python 3.8 has removed the cgi.escape function, which had been deprecated since version 3.2. html.escape does the same thing, use that instead. Signed-off-by: Keith Packard --- test/normalize.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/normalize.py b/test/normalize.py index 6073bf011..e9e6320b8 100644 --- a/test/normalize.py +++ b/test/normalize.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- from html.parser import HTMLParser import urllib +import html try: from html.parser import HTMLParseError @@ -66,7 +67,7 @@ def handle_starttag(self, tag, attrs): self.output += ("=" + '"' + urllib.quote(urllib.unquote(v), safe='/') + '"') elif v != None: - self.output += ("=" + '"' + cgi.escape(v,quote=True) + '"') + self.output += ("=" + '"' + html.escape(v,quote=True) + '"') self.output += ">" self.last_tag = tag self.last = "starttag" From 4bab14a5e2537704868acaf6998a34d2d969b498 Mon Sep 17 00:00:00 2001 From: Phill MV Date: Thu, 12 Aug 2021 09:21:08 -0400 Subject: [PATCH 05/36] Removed outdated build status badges. --- README.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/README.md b/README.md index a36d6f9b3..aafe55ed5 100644 --- a/README.md +++ b/README.md @@ -1,9 +1,6 @@ cmark-gfm ========= -[![Build Status]](https://travis-ci.org/github/cmark-gfm) -[![Windows Build Status]](https://ci.appveyor.com/project/github/cmark) - `cmark-gfm` is an extended version of the C reference implementation of [CommonMark], a rationalized version of Markdown syntax with a spec. This repository adds GitHub Flavored Markdown extensions to From 4a7186e3eb95d40c1bbbdbc3cda5912aa671320a Mon Sep 17 00:00:00 2001 From: Phill MV Date: Thu, 12 Aug 2021 09:28:54 -0400 Subject: [PATCH 06/36] Run ci build only on push, thanks. --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7b7dd1d67..4c0ad9406 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,6 +1,6 @@ name: CI tests -on: [push, pull_request, workflow_dispatch] +on: [push, workflow_dispatch] jobs: linux: From 71e27f25f11c9a34f0532dba459940e1fb5f6316 Mon Sep 17 00:00:00 2001 From: Phill MV Date: Thu, 19 Aug 2021 09:15:56 -0400 Subject: [PATCH 07/36] Footnotes now support being nested, i.e. a footnote may reference another footnote. --- src/blocks.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/blocks.c b/src/blocks.c index 53e882f19..ec5bbe98c 100644 --- a/src/blocks.c +++ b/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,8 +514,10 @@ 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; } From 1f026ef38bea7636b49c31925ecf95af9bb8834f Mon Sep 17 00:00:00 2001 From: Phill MV Date: Thu, 19 Aug 2021 09:22:54 -0400 Subject: [PATCH 08/36] Fix for footnotes being confused for link references. When two footnote references are adjacent, the handle_close_bracket function will first try to match the closing bracket to a link reference. Now we reset the subject's state, so that the parser correctly picks up both footnote references. --- src/inlines.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/inlines.c b/src/inlines.c index c21430bde..bf85dc5a2 100644 --- a/src/inlines.c +++ b/src/inlines.c @@ -1141,6 +1141,13 @@ static cmark_node *handle_close_bracket(cmark_parser *parser, subject *subj) { !opener->inl_text->next->next) { cmark_chunk *literal = &opener->inl_text->next->as.literal; if (literal->len > 1 && literal->data[0] == '^') { + + // 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; + 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; From bb117ffa7f0dcccdc4d7773f8585ef07e2402f36 Mon Sep 17 00:00:00 2001 From: Phill MV Date: Thu, 19 Aug 2021 09:31:02 -0400 Subject: [PATCH 09/36] Fix for when footnote reference labels get broken up into multiple cmark_nodes. Sometimes, the autolinker will go ahead and greedily split input into multiple text nodes in the hopes of matching a hyperlink. This broke footnotes, which expected a singular node. Instead of relying on the tokenizing to have worked perfectly, when handling footnote references we now simply insert the reference based on the closing bracket and ignore and delete any existing and superfluous nodes. --- src/inlines.c | 65 +++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 55 insertions(+), 10 deletions(-) diff --git a/src/inlines.c b/src/inlines.c index c21430bde..02b75fa7f 100644 --- a/src/inlines.c +++ b/src/inlines.c @@ -1137,17 +1137,62 @@ 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); + + // 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->data[0] == '^' && (literal->len > 1 || opener->inl_text->next->next)) { + + 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 + fnref->as.literal = cmark_chunk_dup(literal, 1, (fnref_end_column - fnref_start_column) - 2); + + 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); + + // 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); process_emphasis(parser, subj, opener->previous_delimiter); pop_bracket(subj); From bf76871f492b2e76236bcd817b2ef5d501fa0beb Mon Sep 17 00:00:00 2001 From: Phill MV Date: Thu, 19 Aug 2021 09:44:19 -0400 Subject: [PATCH 10/36] Fix footnote reference label text, and add multiple backrefs. When a footnote is referenced multiple times, we now insert multiple backrefs linking back to each reference. In order to do this, we had to change how footnote ref link labels work away from an incrementing index, and instead use footnote reference label text *plus* an index. --- src/blocks.c | 11 +++++++++++ src/commonmark.c | 10 ++++++---- src/html.c | 51 ++++++++++++++++++++++++++++++++++-------------- src/node.h | 5 +++++ 4 files changed, 58 insertions(+), 19 deletions(-) diff --git a/src/blocks.c b/src/blocks.c index 53e882f19..d70551c08 100644 --- a/src/blocks.c +++ b/src/blocks.c @@ -485,6 +485,17 @@ static void process_footnotes(cmark_parser *parser) { if (!footnote->ix) footnote->ix = ++ix; + // keep track of a) how many times this footnote def has been + // referenced, and b) which reference count this footnote ref is at + // this is used by renderers when generating links and backreferences. + cur->footnote.ix = ++footnote->node->footnote.count; + + // store the footnote reference text label in the footnote ref's node's + // `user_data`, so that renderers can use the label when generating + // links and backreferences. + cur->user_data = parser->mem->calloc(1, (sizeof(char) * cur->as.literal.len) + 1); + memmove(cur->user_data, cur->as.literal.data, cur->as.literal.len); + char n[32]; snprintf(n, sizeof(n), "%d", footnote->ix); cmark_chunk_free(parser->mem, &cur->as.literal); diff --git a/src/commonmark.c b/src/commonmark.c index f272d4d29..a368474f3 100644 --- a/src/commonmark.c +++ b/src/commonmark.c @@ -477,7 +477,7 @@ static int S_render_node(cmark_renderer *renderer, cmark_node *node, case CMARK_NODE_FOOTNOTE_REFERENCE: if (entering) { LIT("[^"); - OUT(cmark_chunk_to_cstr(renderer->mem, &node->as.literal), false, LITERAL); + OUT(node->user_data, false, LITERAL); LIT("]"); } break; @@ -486,9 +486,11 @@ static int S_render_node(cmark_renderer *renderer, cmark_node *node, if (entering) { renderer->footnote_ix += 1; LIT("[^"); - char n[32]; - snprintf(n, sizeof(n), "%d", renderer->footnote_ix); - OUT(n, false, LITERAL); + + char *str = renderer->mem->calloc(1, (sizeof(char) * node->as.literal.len) + 1); + memmove(str, node->as.literal.data, node->as.literal.len); + + OUT(str, false, LITERAL); LIT("]:\n"); cmark_strbuf_puts(renderer->prefix, " "); diff --git a/src/html.c b/src/html.c index ea1f6e189..a70271336 100644 --- a/src/html.c +++ b/src/html.c @@ -59,17 +59,31 @@ static void filter_html_block(cmark_html_renderer *renderer, uint8_t *data, size cmark_strbuf_put(html, data, (bufsize_t)len); } -static bool S_put_footnote_backref(cmark_html_renderer *renderer, cmark_strbuf *html) { +static bool S_put_footnote_backref(cmark_html_renderer *renderer, cmark_strbuf *html, cmark_node *node) { if (renderer->written_footnote_ix >= renderer->footnote_ix) return false; renderer->written_footnote_ix = renderer->footnote_ix; - cmark_strbuf_puts(html, "footnote_ix); - cmark_strbuf_puts(html, n); + cmark_strbuf_puts(html, "as.literal.data, node->as.literal.len); cmark_strbuf_puts(html, "\" class=\"footnote-backref\">↩"); + if (node->footnote.count > 1) + { + for(int i = 2; i <= node->footnote.count; i++) { + char n[32]; + snprintf(n, sizeof(n), "%d", i); + + cmark_strbuf_puts(html, " as.literal.data, node->as.literal.len); + cmark_strbuf_puts(html, ":"); + cmark_strbuf_put(html, (const unsigned char *)n, strlen(n)); + cmark_strbuf_puts(html, "\" class=\"footnote-backref\">↩"); + cmark_strbuf_put(html, (const unsigned char *)n, strlen(n)); + cmark_strbuf_puts(html, ""); + } + } + return true; } @@ -273,7 +287,7 @@ static int S_render_node(cmark_html_renderer *renderer, cmark_node *node, } else { if (parent->type == CMARK_NODE_FOOTNOTE_DEFINITION && node->next == NULL) { cmark_strbuf_putc(html, ' '); - S_put_footnote_backref(renderer, html); + S_put_footnote_backref(renderer, html, parent); } cmark_strbuf_puts(html, "

\n"); } @@ -395,13 +409,12 @@ static int S_render_node(cmark_html_renderer *renderer, cmark_node *node, cmark_strbuf_puts(html, "
\n
    \n"); } ++renderer->footnote_ix; - cmark_strbuf_puts(html, "
  1. footnote_ix); - cmark_strbuf_puts(html, n); + + cmark_strbuf_puts(html, "
  2. as.literal.data, node->as.literal.len); cmark_strbuf_puts(html, "\">\n"); } else { - if (S_put_footnote_backref(renderer, html)) { + if (S_put_footnote_backref(renderer, html, node)) { cmark_strbuf_putc(html, '\n'); } cmark_strbuf_puts(html, "
    3. \n"); @@ -410,10 +423,18 @@ static int S_render_node(cmark_html_renderer *renderer, cmark_node *node, case CMARK_NODE_FOOTNOTE_REFERENCE: if (entering) { - cmark_strbuf_puts(html, "as.literal.data, node->as.literal.len); - cmark_strbuf_puts(html, "\" id=\"fnref"); - cmark_strbuf_put(html, node->as.literal.data, node->as.literal.len); + cmark_strbuf_puts(html, "user_data); + cmark_strbuf_puts(html, "\" id=\"fnref:"); + cmark_strbuf_puts(html, node->user_data); + + if (node->footnote.ix > 1) { + char n[32]; + snprintf(n, sizeof(n), "%d", node->footnote.ix); + cmark_strbuf_puts(html, ":"); + cmark_strbuf_put(html, (const unsigned char *)n, strlen(n)); + } + cmark_strbuf_puts(html, "\">"); cmark_strbuf_put(html, node->as.literal.data, node->as.literal.len); cmark_strbuf_puts(html, ""); diff --git a/src/node.h b/src/node.h index 6391db9f9..c7cdc0f55 100644 --- a/src/node.h +++ b/src/node.h @@ -76,6 +76,11 @@ struct cmark_node { cmark_syntax_extension *extension; + union { + int ix; + int count; + } footnote; + union { cmark_chunk literal; cmark_list list; From 272c99984f370cc3104d00c66757edb78480df43 Mon Sep 17 00:00:00 2001 From: Phill MV Date: Thu, 19 Aug 2021 09:50:16 -0400 Subject: [PATCH 11/36] Fixed footnote extension test to handle new footnote reference link labels. --- test/extensions.txt | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/test/extensions.txt b/test/extensions.txt index 0d9993782..59f74a3d3 100644 --- a/test/extensions.txt +++ b/test/extensions.txt @@ -672,31 +672,31 @@ Hi! [^unused]: This is unused. . -

    This is some text!1. Other text.2.

    -

    Here's a thing3.

    -

    And another thing4.

    +

    This is some text!1. Other text.2.

    +

    Here's a thing3.

    +

    And another thing4.

    This doesn't have a referent[^nope].

    Hi!

      -
    1. -

      Some bolded footnote definition. ↩

      +
    2. +

      Some bolded footnote definition. ↩

      3. -
    3. +

    4. Blockquotes can be in a footnote.

      as well as code blocks
      -

      or, naturally, simple paragraphs. ↩

      +

      or, naturally, simple paragraphs. ↩

      5. -
    5. -

      no code block here (spaces are stripped away) ↩

      +
    6. +

      no code block here (spaces are stripped away) ↩

      7. -
    7. +
    8. this is now a code block (8 spaces indentation)
      -↩ +↩
    From 2cb2f7c88d6b78ef769bc8875c5c3fdb1b9c828c Mon Sep 17 00:00:00 2001 From: Phill MV Date: Thu, 19 Aug 2021 09:50:57 -0400 Subject: [PATCH 12/36] Added test example that exercises a single footnote being referenced in multiple places and multiple backrefs. --- test/extensions.txt | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/extensions.txt b/test/extensions.txt index 59f74a3d3..eed698c79 100644 --- a/test/extensions.txt +++ b/test/extensions.txt @@ -702,6 +702,26 @@ Hi!
```````````````````````````````` +## When a footnote is used multiple times, we insert multiple backrefs. + +```````````````````````````````` example +This is some text. It has a footnote[^a-footnote]. + +This footnote is referenced[^a-footnote] multiple times, in lots of different places.[^a-footnote] + +[^a-footnote]: This footnote definition should have three backrefs. +. +

This is some text. It has a footnote1.

+

This footnote is referenced1 multiple times, in lots of different places.1

+
+
    +
  1. +

    This footnote definition should have three backrefs. ↩ ↩2 ↩3

    +
    2. +
+
+```````````````````````````````` + ## Interop Autolink and strikethrough. From 8ccdaa7d1cf65501735220366c1316474ff7e893 Mon Sep 17 00:00:00 2001 From: Phill MV Date: Thu, 19 Aug 2021 09:56:10 -0400 Subject: [PATCH 13/36] Converted regression test to expect new footnote ref link labels. --- test/regression.txt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/regression.txt b/test/regression.txt index d033a4003..ae0c8be40 100644 --- a/test/regression.txt +++ b/test/regression.txt @@ -175,7 +175,7 @@ A footnote in a paragraph[^1] [^1]: a footnote . -

A footnote in a paragraph1

+

A footnote in a paragraph1

@@ -185,15 +185,15 @@ A footnote in a paragraph[^1] - +
foot 1foot 1 note
    -
  1. -

    a footnote ↩

    +
  2. +

    a footnote ↩ ↩2

From a0de7d891c350b306d0e8188f49a1a4c72824a9a Mon Sep 17 00:00:00 2001 From: Phill MV Date: Fri, 20 Aug 2021 10:15:40 -0400 Subject: [PATCH 14/36] Added regression test that exercises nested footnotes. --- test/regression.txt | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/regression.txt b/test/regression.txt index d033a4003..7cc10c0d2 100644 --- a/test/regression.txt +++ b/test/regression.txt @@ -269,3 +269,26 @@ Pull request #128 - Buffer overread in tables extension

| -|

```````````````````````````````` + +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] +. +

This is some text. It has a citation.1

+
+
    +
  1. +

    This is a long winded parapgraph that also has another citation.2 ↩

    +
    2. +
  2. +

    My second citation. ↩

    +
    3. +
+
+```````````````````````````````` From 7fa237247371bb3067bcb8fcb4ce74bc95ac04e0 Mon Sep 17 00:00:00 2001 From: Phill MV Date: Fri, 20 Aug 2021 10:18:16 -0400 Subject: [PATCH 15/36] Added test that properly exercises footnotes whose reference labels contain 'w' or '_'. --- test/regression.txt | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/regression.txt b/test/regression.txt index d033a4003..f0e99b4a6 100644 --- a/test/regression.txt +++ b/test/regression.txt @@ -269,3 +269,28 @@ Pull request #128 - Buffer overread in tables extension

| -|

```````````````````````````````` + +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. +. +

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.1

+

It has another footnote that contains many different characters (the autolinker was also breaking on _).2

+
+
    +
  1. +

    this renders properly. ↩

    +
    2. +
  2. +

    so does this. ↩

    +
    3. +
+
+```````````````````````````````` From 740b98704fbd93d1a7dee46077964db7df14263d Mon Sep 17 00:00:00 2001 From: Phill MV Date: Fri, 20 Aug 2021 10:21:45 -0400 Subject: [PATCH 16/36] Added test that exercises whether footnotes are confused for link references. --- test/regression.txt | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/regression.txt b/test/regression.txt index d033a4003..85c79507b 100644 --- a/test/regression.txt +++ b/test/regression.txt @@ -269,3 +269,25 @@ Pull request #128 - Buffer overread in tables extension

| -|

```````````````````````````````` + +## 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. +. +

This is some text. It has two footnotes references, side-by-side without any spaces,12 which are definitely not link references.

+
+
    +
  1. +

    Hello. ↩

    +
    2. +
  2. +

    Goodbye. ↩

    +
    3. +
+
+```````````````````````````````` From 7b5d45d05678504d8b62d80bec8e78cf0358b343 Mon Sep 17 00:00:00 2001 From: Phill MV Date: Fri, 20 Aug 2021 11:42:05 -0400 Subject: [PATCH 17/36] Adapted existing regression tests to conform to new footnote ref label. --- test/regression.txt | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/test/regression.txt b/test/regression.txt index 37d795394..47bd104f5 100644 --- a/test/regression.txt +++ b/test/regression.txt @@ -279,14 +279,14 @@ This is some text. It has a citation.[^citation] [^citation]: This is a long winded parapgraph that also has another citation.[^another-citation] . -

This is some text. It has a citation.1

+

This is some text. It has a citation.1

    -
  1. -

    This is a long winded parapgraph that also has another citation.2 ↩

    +
  2. +

    This is a long winded parapgraph that also has another citation.2 ↩

    3. -
  3. -

    My second citation. ↩

    +
  4. +

    My second citation. ↩

@@ -301,14 +301,14 @@ This is some text. It has two footnotes references, side-by-side without any spa [^footnote2]: Goodbye. . -

This is some text. It has two footnotes references, side-by-side without any spaces,12 which are definitely not link references.

+

This is some text. It has two footnotes references, side-by-side without any spaces,12 which are definitely not link references.

    -
  1. -

    Hello. ↩

    +
  2. +

    Hello. ↩

    3. -
  3. -

    Goodbye. ↩

    +
  4. +

    Goodbye. ↩

@@ -325,15 +325,15 @@ It has another footnote that contains many different characters (the autolinker [^widely-cited]: this renders properly. . -

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.1

-

It has another footnote that contains many different characters (the autolinker was also breaking on _).2

+

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.1

+

It has another footnote that contains many different characters (the autolinker was also breaking on _).2

    -
  1. -

    this renders properly. ↩

    +
  2. +

    this renders properly. ↩

    3. -
  3. -

    so does this. ↩

    +
  4. +

    so does this. ↩

From fdd7851bbc7238022a6c3d6221ad2d9541435352 Mon Sep 17 00:00:00 2001 From: Phill MV Date: Mon, 23 Aug 2021 14:35:47 -0400 Subject: [PATCH 18/36] Added Actions CI badge. --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index aafe55ed5..0b9f719f6 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,8 @@ cmark-gfm ========= +![Actions CI](https://github.com/github/cmark-gfm/actions/workflows/ci.yml/badge.svg) + `cmark-gfm` is an extended version of the C reference implementation of [CommonMark], a rationalized version of Markdown syntax with a spec. This repository adds GitHub Flavored Markdown extensions to From 02776382422fd97986681458aa6c156a7fdcc652 Mon Sep 17 00:00:00 2001 From: Phill MV Date: Mon, 23 Aug 2021 14:37:16 -0400 Subject: [PATCH 19/36] Removed redundant 'import cgi' from normalize.py --- test/normalize.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/normalize.py b/test/normalize.py index e9e6320b8..b7fd9b245 100644 --- a/test/normalize.py +++ b/test/normalize.py @@ -14,7 +14,6 @@ class HTMLParseError(Exception): from html.entities import name2codepoint import sys import re -import cgi # Normalization code, adapted from # https://github.com/karlcow/markdown-testsuite/ From b790eca090237255ab1e60c1dfb2ec5f67ba5dcd Mon Sep 17 00:00:00 2001 From: Phill MV Date: Wed, 25 Aug 2021 10:41:58 -0400 Subject: [PATCH 20/36] Bumped version to 0.29.0.gfm.1 --- CMakeLists.txt | 2 +- changelog.txt | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c92bde52d..34773819f 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -4,7 +4,7 @@ project(cmark-gfm) set(PROJECT_VERSION_MAJOR 0) set(PROJECT_VERSION_MINOR 29) set(PROJECT_VERSION_PATCH 0) -set(PROJECT_VERSION_GFM 0) +set(PROJECT_VERSION_GFM 1) set(PROJECT_VERSION ${PROJECT_VERSION_MAJOR}.${PROJECT_VERSION_MINOR}.${PROJECT_VERSION_PATCH}.gfm.${PROJECT_VERSION_GFM}) include("FindAsan.cmake") diff --git a/changelog.txt b/changelog.txt index b86a41a22..5ba1e62b3 100644 --- a/changelog.txt +++ b/changelog.txt @@ -1,3 +1,8 @@ +[0.29.0.gfm.1] + + * Fixed denial of service bug in GFM's table extension + per https://github.com/github/cmark-gfm/security/advisories/GHSA-7gc6-9qr5-hc85 + [0.29.0] * Update spec to 0.29. From 32ffc7719e72c39422ebb3c161f28aa553799f30 Mon Sep 17 00:00:00 2001 From: Phill MV Date: Wed, 1 Sep 2021 10:48:27 -0400 Subject: [PATCH 21/36] Renamed cmark_node->footnote.{ix,count} to {ref_ix,def_count} to make intent more obvious. --- src/blocks.c | 6 +++--- src/html.c | 8 ++++---- src/node.h | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/blocks.c b/src/blocks.c index 836de2c8e..5d43ce339 100644 --- a/src/blocks.c +++ b/src/blocks.c @@ -484,10 +484,10 @@ static void process_footnotes(cmark_parser *parser) { if (!footnote->ix) footnote->ix = ++ix; - // keep track of a) how many times this footnote def has been - // referenced, and b) which reference count this footnote ref is at + // keep track of a) count of how many times this footnote def has been + // referenced, and b) which reference index this footnote ref is at. // this is used by renderers when generating links and backreferences. - cur->footnote.ix = ++footnote->node->footnote.count; + cur->footnote.ref_ix = ++footnote->node->footnote.def_count; // store the footnote reference text label in the footnote ref's node's // `user_data`, so that renderers can use the label when generating diff --git a/src/html.c b/src/html.c index a70271336..0e3ecb36d 100644 --- a/src/html.c +++ b/src/html.c @@ -68,9 +68,9 @@ static bool S_put_footnote_backref(cmark_html_renderer *renderer, cmark_strbuf * cmark_strbuf_put(html, node->as.literal.data, node->as.literal.len); cmark_strbuf_puts(html, "\" class=\"footnote-backref\">↩"); - if (node->footnote.count > 1) + if (node->footnote.def_count > 1) { - for(int i = 2; i <= node->footnote.count; i++) { + for(int i = 2; i <= node->footnote.def_count; i++) { char n[32]; snprintf(n, sizeof(n), "%d", i); @@ -428,9 +428,9 @@ static int S_render_node(cmark_html_renderer *renderer, cmark_node *node, cmark_strbuf_puts(html, "\" id=\"fnref:"); cmark_strbuf_puts(html, node->user_data); - if (node->footnote.ix > 1) { + if (node->footnote.ref_ix > 1) { char n[32]; - snprintf(n, sizeof(n), "%d", node->footnote.ix); + snprintf(n, sizeof(n), "%d", node->footnote.ref_ix); cmark_strbuf_puts(html, ":"); cmark_strbuf_put(html, (const unsigned char *)n, strlen(n)); } diff --git a/src/node.h b/src/node.h index c7cdc0f55..e158e908c 100644 --- a/src/node.h +++ b/src/node.h @@ -77,8 +77,8 @@ struct cmark_node { cmark_syntax_extension *extension; union { - int ix; - int count; + int ref_ix; + int def_count; } footnote; union { From 17170400954862dc0a626ae7467aaef6f6f78448 Mon Sep 17 00:00:00 2001 From: Phill MV Date: Wed, 1 Sep 2021 11:52:53 -0400 Subject: [PATCH 22/36] Added cmark_node.parent_footnote_def, removed usage of 'user_data', made sure to free allocated string in commonmark.c --- src/blocks.c | 10 ++++------ src/commonmark.c | 16 ++++++++++++---- src/html.c | 4 ++-- src/node.h | 2 ++ 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/blocks.c b/src/blocks.c index 5d43ce339..e19b7e905 100644 --- a/src/blocks.c +++ b/src/blocks.c @@ -484,17 +484,15 @@ static void process_footnotes(cmark_parser *parser) { if (!footnote->ix) footnote->ix = ++ix; + // store a reference to this footnote reference's footnote definition + // this is used by renderers when generating label ids + cur->parent_footnote_def = footnote->node; + // keep track of a) count of how many times this footnote def has been // referenced, and b) which reference index this footnote ref is at. // this is used by renderers when generating links and backreferences. cur->footnote.ref_ix = ++footnote->node->footnote.def_count; - // store the footnote reference text label in the footnote ref's node's - // `user_data`, so that renderers can use the label when generating - // links and backreferences. - cur->user_data = parser->mem->calloc(1, (sizeof(char) * cur->as.literal.len) + 1); - memmove(cur->user_data, cur->as.literal.data, cur->as.literal.len); - char n[32]; snprintf(n, sizeof(n), "%d", footnote->ix); cmark_chunk_free(parser->mem, &cur->as.literal); diff --git a/src/commonmark.c b/src/commonmark.c index a368474f3..8fe03687b 100644 --- a/src/commonmark.c +++ b/src/commonmark.c @@ -477,7 +477,13 @@ static int S_render_node(cmark_renderer *renderer, cmark_node *node, case CMARK_NODE_FOOTNOTE_REFERENCE: if (entering) { LIT("[^"); - OUT(node->user_data, false, LITERAL); + + char *footnote_label = renderer->mem->calloc(1, (sizeof(char) * node->parent_footnote_def->as.literal.len) + 1); + memmove(footnote_label, node->parent_footnote_def->as.literal.data, node->parent_footnote_def->as.literal.len); + + OUT(footnote_label, false, LITERAL); + renderer->mem->free(footnote_label); + LIT("]"); } break; @@ -487,10 +493,12 @@ static int S_render_node(cmark_renderer *renderer, cmark_node *node, renderer->footnote_ix += 1; LIT("[^"); - char *str = renderer->mem->calloc(1, (sizeof(char) * node->as.literal.len) + 1); - memmove(str, node->as.literal.data, node->as.literal.len); + char *footnote_label = renderer->mem->calloc(1, (sizeof(char) * node->as.literal.len) + 1); + memmove(footnote_label, node->as.literal.data, node->as.literal.len); + + OUT(footnote_label, false, LITERAL); + renderer->mem->free(footnote_label); - OUT(str, false, LITERAL); LIT("]:\n"); cmark_strbuf_puts(renderer->prefix, " "); diff --git a/src/html.c b/src/html.c index 0e3ecb36d..a3fe23802 100644 --- a/src/html.c +++ b/src/html.c @@ -424,9 +424,9 @@ static int S_render_node(cmark_html_renderer *renderer, cmark_node *node, case CMARK_NODE_FOOTNOTE_REFERENCE: if (entering) { cmark_strbuf_puts(html, "user_data); + cmark_strbuf_put(html, node->parent_footnote_def->as.literal.data, node->parent_footnote_def->as.literal.len); cmark_strbuf_puts(html, "\" id=\"fnref:"); - cmark_strbuf_puts(html, node->user_data); + cmark_strbuf_put(html, node->parent_footnote_def->as.literal.data, node->parent_footnote_def->as.literal.len); if (node->footnote.ref_ix > 1) { char n[32]; diff --git a/src/node.h b/src/node.h index e158e908c..b094c16e6 100644 --- a/src/node.h +++ b/src/node.h @@ -81,6 +81,8 @@ struct cmark_node { int def_count; } footnote; + cmark_node *parent_footnote_def; + union { cmark_chunk literal; cmark_list list; From 32002eca5831e09a1911cb85d6539e0c481a09cd Mon Sep 17 00:00:00 2001 From: Phill MV Date: Wed, 1 Sep 2021 12:18:22 -0400 Subject: [PATCH 23/36] replaced strbuf_put with strbuf_puts --- src/html.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/html.c b/src/html.c index a3fe23802..a29218755 100644 --- a/src/html.c +++ b/src/html.c @@ -77,9 +77,9 @@ static bool S_put_footnote_backref(cmark_html_renderer *renderer, cmark_strbuf * cmark_strbuf_puts(html, " as.literal.data, node->as.literal.len); cmark_strbuf_puts(html, ":"); - cmark_strbuf_put(html, (const unsigned char *)n, strlen(n)); + cmark_strbuf_puts(html, n); cmark_strbuf_puts(html, "\" class=\"footnote-backref\">↩"); - cmark_strbuf_put(html, (const unsigned char *)n, strlen(n)); + cmark_strbuf_puts(html, n); cmark_strbuf_puts(html, ""); } } @@ -432,7 +432,7 @@ static int S_render_node(cmark_html_renderer *renderer, cmark_node *node, char n[32]; snprintf(n, sizeof(n), "%d", node->footnote.ref_ix); cmark_strbuf_puts(html, ":"); - cmark_strbuf_put(html, (const unsigned char *)n, strlen(n)); + cmark_strbuf_puts(html, n); } cmark_strbuf_puts(html, "\">"); From 6e186b307a2addf2d5507de5e35f0d59c7e1c28d Mon Sep 17 00:00:00 2001 From: Phill MV Date: Wed, 1 Sep 2021 20:02:34 -0400 Subject: [PATCH 24/36] WIP: what if we only free the nodes after calling process_emphasis? --- src/inlines.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/inlines.c b/src/inlines.c index 98dc17c39..057bf39b7 100644 --- a/src/inlines.c +++ b/src/inlines.c @@ -1191,6 +1191,8 @@ static cmark_node *handle_close_bracket(cmark_parser *parser, subject *subj) { // the opener->inl_text->next. // // therefore, here we walk thru the list and free them all up + process_emphasis(parser, subj, opener->previous_delimiter); + cmark_node *next_node; cmark_node *current_node = opener->inl_text->next; while(current_node) { @@ -1200,7 +1202,7 @@ static cmark_node *handle_close_bracket(cmark_parser *parser, subject *subj) { } cmark_node_free(opener->inl_text); - process_emphasis(parser, subj, opener->previous_delimiter); + pop_bracket(subj); return NULL; } From d3a819ce43d60c26ad0656413e64b50908559cfb Mon Sep 17 00:00:00 2001 From: Phill MV Date: Thu, 2 Sep 2021 10:12:42 -0400 Subject: [PATCH 25/36] Fix & regression test for use-after-free introduced in bb117ffa7f0dcccdc4d7773f8585ef07e2402f36 tldr, avoid freeing memory before passing it along to another function. --- src/inlines.c | 3 +-- test/regression.txt | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/inlines.c b/src/inlines.c index 057bf39b7..d4c936531 100644 --- a/src/inlines.c +++ b/src/inlines.c @@ -1178,6 +1178,7 @@ static cmark_node *handle_close_bracket(cmark_parser *parser, subject *subj) { // 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 @@ -1191,8 +1192,6 @@ static cmark_node *handle_close_bracket(cmark_parser *parser, subject *subj) { // the opener->inl_text->next. // // therefore, here we walk thru the list and free them all up - process_emphasis(parser, subj, opener->previous_delimiter); - cmark_node *next_node; cmark_node *current_node = opener->inl_text->next; while(current_node) { diff --git a/test/regression.txt b/test/regression.txt index ea2eadd97..d4d921a51 100644 --- a/test/regression.txt +++ b/test/regression.txt @@ -338,3 +338,19 @@ It has another footnote that contains many different characters (the autolinker ```````````````````````````````` + +Footnotes interacting with strikethrough should not lead to a use-after-free + +```````````````````````````````` example footnotes autolink strikethrough table +|Tot.....[^_a_]| +. +

|Tot.....[^_a_]|

+```````````````````````````````` + +Footnotes interacting with strikethrough should not lead to a use-after-free pt2 + +```````````````````````````````` example footnotes autolink strikethrough table +[^~~is~~1] +. +

[^~~is~~1]

+```````````````````````````````` From a1d171ad498757e4fed55aff7fb28e5665af983a Mon Sep 17 00:00:00 2001 From: Phill MV Date: Thu, 2 Sep 2021 09:58:08 -0400 Subject: [PATCH 26/36] Fix for use-after-free bug introduced in 71e27f25f11c9a34f0532dba459940e1fb5f6316 Sometimes, when cleaning up unusued footnotes, two footnote->nodes may end up referencing each other. As they get free()'d up, this can lead to problems. Instead, first we unlink every node and _then_ free them up. --- src/blocks.c | 1 + src/footnotes.c | 23 +++++++++++++++++++++++ src/footnotes.h | 2 ++ test/regression.txt | 11 +++++++++++ 4 files changed, 37 insertions(+) diff --git a/src/blocks.c b/src/blocks.c index ec5bbe98c..1c22c95e6 100644 --- a/src/blocks.c +++ b/src/blocks.c @@ -523,6 +523,7 @@ static void process_footnotes(cmark_parser *parser) { } } + cmark_unlink_footnotes_map(map); cmark_map_free(map); } diff --git a/src/footnotes.c b/src/footnotes.c index f2d2765f4..c2b745f79 100644 --- a/src/footnotes.c +++ b/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; + } +} diff --git a/src/footnotes.h b/src/footnotes.h index 43dd64ff6..64e2901e3 100644 --- a/src/footnotes.h +++ b/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 diff --git a/test/regression.txt b/test/regression.txt index d4d921a51..f7e984379 100644 --- a/test/regression.txt +++ b/test/regression.txt @@ -354,3 +354,14 @@ Footnotes interacting with strikethrough should not lead to a use-after-free pt2 .

[^~~is~~1]

```````````````````````````````` + +Adjacent unused footnotes definitions should not lead to a use after free + +```````````````````````````````` example footnotes autolink strikethrough table +Hello world + + +[^a]:[^b]: +. +

Hello world

+```````````````````````````````` From d43ae4b274b73f1e023f9f1f150deea05dbe64ba Mon Sep 17 00:00:00 2001 From: Phill MV Date: Tue, 14 Sep 2021 17:17:53 -0400 Subject: [PATCH 27/36] By default, always escape footnote hrefs when emitting html. --- src/html.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/html.c b/src/html.c index a29218755..f87acd285 100644 --- a/src/html.c +++ b/src/html.c @@ -65,7 +65,7 @@ static bool S_put_footnote_backref(cmark_html_renderer *renderer, cmark_strbuf * renderer->written_footnote_ix = renderer->footnote_ix; cmark_strbuf_puts(html, "as.literal.data, node->as.literal.len); + houdini_escape_href(html, node->as.literal.data, node->as.literal.len); cmark_strbuf_puts(html, "\" class=\"footnote-backref\">↩"); if (node->footnote.def_count > 1) @@ -75,7 +75,7 @@ static bool S_put_footnote_backref(cmark_html_renderer *renderer, cmark_strbuf * snprintf(n, sizeof(n), "%d", i); cmark_strbuf_puts(html, " as.literal.data, node->as.literal.len); + houdini_escape_href(html, node->as.literal.data, node->as.literal.len); cmark_strbuf_puts(html, ":"); cmark_strbuf_puts(html, n); cmark_strbuf_puts(html, "\" class=\"footnote-backref\">↩"); @@ -411,7 +411,7 @@ static int S_render_node(cmark_html_renderer *renderer, cmark_node *node, ++renderer->footnote_ix; cmark_strbuf_puts(html, "
  • as.literal.data, node->as.literal.len); + houdini_escape_href(html, node->as.literal.data, node->as.literal.len); cmark_strbuf_puts(html, "\">\n"); } else { if (S_put_footnote_backref(renderer, html, node)) { @@ -424,9 +424,9 @@ static int S_render_node(cmark_html_renderer *renderer, cmark_node *node, case CMARK_NODE_FOOTNOTE_REFERENCE: if (entering) { cmark_strbuf_puts(html, "parent_footnote_def->as.literal.data, node->parent_footnote_def->as.literal.len); + houdini_escape_href(html, node->parent_footnote_def->as.literal.data, node->parent_footnote_def->as.literal.len); cmark_strbuf_puts(html, "\" id=\"fnref:"); - cmark_strbuf_put(html, node->parent_footnote_def->as.literal.data, node->parent_footnote_def->as.literal.len); + houdini_escape_href(html, node->parent_footnote_def->as.literal.data, node->parent_footnote_def->as.literal.len); if (node->footnote.ref_ix > 1) { char n[32]; @@ -436,7 +436,7 @@ static int S_render_node(cmark_html_renderer *renderer, cmark_node *node, } cmark_strbuf_puts(html, "\">"); - cmark_strbuf_put(html, node->as.literal.data, node->as.literal.len); + houdini_escape_href(html, node->as.literal.data, node->as.literal.len); cmark_strbuf_puts(html, ""); } break; From a86bbc5a6cb3fc5c0e632b33281760b51ea497c8 Mon Sep 17 00:00:00 2001 From: Phill MV Date: Wed, 15 Sep 2021 09:28:20 -0400 Subject: [PATCH 28/36] added extension test that verifies that footnote labels get href escaped. --- test/extensions.txt | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test/extensions.txt b/test/extensions.txt index eed698c79..d7b40ca80 100644 --- a/test/extensions.txt +++ b/test/extensions.txt @@ -722,6 +722,23 @@ This footnote is referenced[^a-footnote] multiple times, in lots of different pl ```````````````````````````````` +## Footnote reference labels are href escaped + +```````````````````````````````` example +Hello[^">] + +[^">]: pwned +. +

    Hello1

    +
    +
      +
    1. +

      pwned ↩

      +
      2. +
    +
    +```````````````````````````````` + ## Interop Autolink and strikethrough. From 586a22d11dbe951045082a4398242ef25b42a8f8 Mon Sep 17 00:00:00 2001 From: Phill MV Date: Wed, 15 Sep 2021 11:08:33 -0400 Subject: [PATCH 29/36] literal->data is probably NULL terminated, but just in case let's check literal->len first. --- src/inlines.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/inlines.c b/src/inlines.c index d4c936531..f14722940 100644 --- a/src/inlines.c +++ b/src/inlines.c @@ -1144,7 +1144,7 @@ static cmark_node *handle_close_bracket(cmark_parser *parser, subject *subj) { // 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->data[0] == '^' && (literal->len > 1 || opener->inl_text->next->next)) { + 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 From de6feae6ed595154de46ca7de66c45ba2954bb9e Mon Sep 17 00:00:00 2001 From: Phill MV Date: Wed, 15 Sep 2021 12:01:28 -0400 Subject: [PATCH 30/36] Added check for underflows when duping footnote ref literal. --- src/inlines.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/inlines.c b/src/inlines.c index f14722940..4a118a636 100644 --- a/src/inlines.c +++ b/src/inlines.c @@ -1168,7 +1168,13 @@ static cmark_node *handle_close_bracket(cmark_parser *parser, subject *subj) { // // this copies the footnote reference string, even if between the // `opener` and the subject's current position there are other nodes - fnref->as.literal = cmark_chunk_dup(literal, 1, (fnref_end_column - fnref_start_column) - 2); + // + // (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; From 4bf57ea384b4b4133b7a0687bcd98e0d71406572 Mon Sep 17 00:00:00 2001 From: Phill MV Date: Wed, 15 Sep 2021 12:18:32 -0400 Subject: [PATCH 31/36] Swapped calloc argument order, so that we use the function appropriately. --- src/commonmark.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/commonmark.c b/src/commonmark.c index 8fe03687b..2e0719443 100644 --- a/src/commonmark.c +++ b/src/commonmark.c @@ -478,7 +478,7 @@ static int S_render_node(cmark_renderer *renderer, cmark_node *node, if (entering) { LIT("[^"); - char *footnote_label = renderer->mem->calloc(1, (sizeof(char) * node->parent_footnote_def->as.literal.len) + 1); + char *footnote_label = renderer->mem->calloc(node->parent_footnote_def->as.literal.len + 1, sizeof(char)); memmove(footnote_label, node->parent_footnote_def->as.literal.data, node->parent_footnote_def->as.literal.len); OUT(footnote_label, false, LITERAL); @@ -493,7 +493,7 @@ static int S_render_node(cmark_renderer *renderer, cmark_node *node, renderer->footnote_ix += 1; LIT("[^"); - char *footnote_label = renderer->mem->calloc(1, (sizeof(char) * node->as.literal.len) + 1); + char *footnote_label = renderer->mem->calloc(node->as.literal.len + 1, sizeof(char)); memmove(footnote_label, node->as.literal.data, node->as.literal.len); OUT(footnote_label, false, LITERAL); From 84742896147b0079b0fc9d4dcf315f68cfcc1772 Mon Sep 17 00:00:00 2001 From: Phill MV Date: Wed, 15 Sep 2021 12:26:47 -0400 Subject: [PATCH 32/36] Swapped : for - when emitting html footnote ref labels. --- src/html.c | 14 +++++++------- test/extensions.txt | 36 ++++++++++++++++++------------------ test/regression.txt | 40 ++++++++++++++++++++-------------------- 3 files changed, 45 insertions(+), 45 deletions(-) diff --git a/src/html.c b/src/html.c index f87acd285..d072798ee 100644 --- a/src/html.c +++ b/src/html.c @@ -64,7 +64,7 @@ static bool S_put_footnote_backref(cmark_html_renderer *renderer, cmark_strbuf * return false; renderer->written_footnote_ix = renderer->footnote_ix; - cmark_strbuf_puts(html, "as.literal.data, node->as.literal.len); cmark_strbuf_puts(html, "\" class=\"footnote-backref\">↩"); @@ -74,9 +74,9 @@ static bool S_put_footnote_backref(cmark_html_renderer *renderer, cmark_strbuf * char n[32]; snprintf(n, sizeof(n), "%d", i); - cmark_strbuf_puts(html, " as.literal.data, node->as.literal.len); - cmark_strbuf_puts(html, ":"); + cmark_strbuf_puts(html, "-"); cmark_strbuf_puts(html, n); cmark_strbuf_puts(html, "\" class=\"footnote-backref\">↩"); cmark_strbuf_puts(html, n); @@ -410,7 +410,7 @@ static int S_render_node(cmark_html_renderer *renderer, cmark_node *node, } ++renderer->footnote_ix; - cmark_strbuf_puts(html, "
  • as.literal.data, node->as.literal.len); cmark_strbuf_puts(html, "\">\n"); } else { @@ -423,15 +423,15 @@ static int S_render_node(cmark_html_renderer *renderer, cmark_node *node, case CMARK_NODE_FOOTNOTE_REFERENCE: if (entering) { - cmark_strbuf_puts(html, "parent_footnote_def->as.literal.data, node->parent_footnote_def->as.literal.len); - cmark_strbuf_puts(html, "\" id=\"fnref:"); + cmark_strbuf_puts(html, "\" id=\"fnref-"); houdini_escape_href(html, node->parent_footnote_def->as.literal.data, node->parent_footnote_def->as.literal.len); if (node->footnote.ref_ix > 1) { char n[32]; snprintf(n, sizeof(n), "%d", node->footnote.ref_ix); - cmark_strbuf_puts(html, ":"); + cmark_strbuf_puts(html, "-"); cmark_strbuf_puts(html, n); } diff --git a/test/extensions.txt b/test/extensions.txt index d7b40ca80..c93e4dedd 100644 --- a/test/extensions.txt +++ b/test/extensions.txt @@ -672,31 +672,31 @@ Hi! [^unused]: This is unused. . -

    This is some text!1. Other text.2.

    -

    Here's a thing3.

    -

    And another thing4.

    +

    This is some text!1. Other text.2.

    +

    Here's a thing3.

    +

    And another thing4.

    This doesn't have a referent[^nope].

    Hi!

      -
    1. -

      Some bolded footnote definition. ↩

      +
    2. +

      Some bolded footnote definition. ↩

      3. -
    3. +

    4. Blockquotes can be in a footnote.

      as well as code blocks
      -

      or, naturally, simple paragraphs. ↩

      +

      or, naturally, simple paragraphs. ↩

      5. -
    5. -

      no code block here (spaces are stripped away) ↩

      +
    6. +

      no code block here (spaces are stripped away) ↩

      7. -
    7. +
    8. this is now a code block (8 spaces indentation)
      -↩ +↩
    @@ -711,12 +711,12 @@ This footnote is referenced[^a-footnote] multiple times, in lots of different pl [^a-footnote]: This footnote definition should have three backrefs. . -

    This is some text. It has a footnote1.

    -

    This footnote is referenced1 multiple times, in lots of different places.1

    +

    This is some text. It has a footnote1.

    +

    This footnote is referenced1 multiple times, in lots of different places.1

      -
    1. -

      This footnote definition should have three backrefs. ↩ ↩2 ↩3

      +
    2. +

      This footnote definition should have three backrefs. ↩ ↩2 ↩3

    @@ -729,11 +729,11 @@ Hello[^">] [^">]: pwned . -

    Hello1

    +

    Hello1

      -
    1. -

      pwned ↩

      +
    2. +

      pwned ↩

    diff --git a/test/regression.txt b/test/regression.txt index fcf5b3e80..e8e593f25 100644 --- a/test/regression.txt +++ b/test/regression.txt @@ -175,7 +175,7 @@ A footnote in a paragraph[^1] [^1]: a footnote . -

    A footnote in a paragraph1

    +

    A footnote in a paragraph1

    @@ -185,15 +185,15 @@ A footnote in a paragraph[^1] - +
    foot 1foot 1 note
      -
    1. -

      a footnote ↩ ↩2

      +
    2. +

      a footnote ↩ ↩2

    @@ -279,14 +279,14 @@ This is some text. It has a citation.[^citation] [^citation]: This is a long winded parapgraph that also has another citation.[^another-citation] . -

    This is some text. It has a citation.1

    +

    This is some text. It has a citation.1

      -
    1. -

      This is a long winded parapgraph that also has another citation.2 ↩

      +
    2. +

      This is a long winded parapgraph that also has another citation.2 ↩

      3. -
    3. -

      My second citation. ↩

      +
    4. +

      My second citation. ↩

    @@ -301,14 +301,14 @@ This is some text. It has two footnotes references, side-by-side without any spa [^footnote2]: Goodbye. . -

    This is some text. It has two footnotes references, side-by-side without any spaces,12 which are definitely not link references.

    +

    This is some text. It has two footnotes references, side-by-side without any spaces,12 which are definitely not link references.

      -
    1. -

      Hello. ↩

      +
    2. +

      Hello. ↩

      3. -
    3. -

      Goodbye. ↩

      +
    4. +

      Goodbye. ↩

    @@ -325,15 +325,15 @@ It has another footnote that contains many different characters (the autolinker [^widely-cited]: this renders properly. . -

    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.1

    -

    It has another footnote that contains many different characters (the autolinker was also breaking on _).2

    +

    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.1

    +

    It has another footnote that contains many different characters (the autolinker was also breaking on _).2

      -
    1. -

      this renders properly. ↩

      +
    2. +

      this renders properly. ↩

      3. -
    3. -

      so does this. ↩

      +
    4. +

      so does this. ↩

    From b6e462f92ff7939bcb2e0ac7812c8ecd58c3e356 Mon Sep 17 00:00:00 2001 From: Tracy Lum <11878752+talum@users.noreply.github.com> Date: Tue, 14 Sep 2021 15:55:34 +0000 Subject: [PATCH 33/36] Add data attributes for footnotes Add data attributes for easier styling within dotcom. Also, add the aria-label for accessibility. --- src/html.c | 8 ++++---- test/extensions.txt | 30 +++++++++++++++--------------- test/regression.txt | 37 +++++++++++++++++++------------------ 3 files changed, 38 insertions(+), 37 deletions(-) diff --git a/src/html.c b/src/html.c index d072798ee..12d3c3e9c 100644 --- a/src/html.c +++ b/src/html.c @@ -66,7 +66,7 @@ static bool S_put_footnote_backref(cmark_html_renderer *renderer, cmark_strbuf * cmark_strbuf_puts(html, "as.literal.data, node->as.literal.len); - cmark_strbuf_puts(html, "\" class=\"footnote-backref\">↩"); + cmark_strbuf_puts(html, "\" class=\"footnote-backref\" data-footnote-backref aria-label=\"Back to content\">↩"); if (node->footnote.def_count > 1) { @@ -78,7 +78,7 @@ static bool S_put_footnote_backref(cmark_html_renderer *renderer, cmark_strbuf * houdini_escape_href(html, node->as.literal.data, node->as.literal.len); cmark_strbuf_puts(html, "-"); cmark_strbuf_puts(html, n); - cmark_strbuf_puts(html, "\" class=\"footnote-backref\">↩"); + cmark_strbuf_puts(html, "\" class=\"footnote-backref\" data-footnote-backref aria-label=\"Back to content\">↩"); cmark_strbuf_puts(html, n); cmark_strbuf_puts(html, ""); } @@ -406,7 +406,7 @@ static int S_render_node(cmark_html_renderer *renderer, cmark_node *node, case CMARK_NODE_FOOTNOTE_DEFINITION: if (entering) { if (renderer->footnote_ix == 0) { - cmark_strbuf_puts(html, "
    \n
      \n"); + cmark_strbuf_puts(html, "
      \n
        \n"); } ++renderer->footnote_ix; @@ -435,7 +435,7 @@ static int S_render_node(cmark_html_renderer *renderer, cmark_node *node, cmark_strbuf_puts(html, n); } - cmark_strbuf_puts(html, "\">"); + cmark_strbuf_puts(html, "\" data-footnote-ref>"); houdini_escape_href(html, node->as.literal.data, node->as.literal.len); cmark_strbuf_puts(html, ""); } diff --git a/test/extensions.txt b/test/extensions.txt index c93e4dedd..37033b1ca 100644 --- a/test/extensions.txt +++ b/test/extensions.txt @@ -672,15 +672,15 @@ Hi! [^unused]: This is unused. . -

        This is some text!1. Other text.2.

        -

        Here's a thing3.

        -

        And another thing4.

        +

        This is some text!1. Other text.2.

        +

        Here's a thing3.

        +

        And another thing4.

        This doesn't have a referent[^nope].

        Hi!

        -
        +
        1. -

          Some bolded footnote definition. ↩

          +

          Some bolded footnote definition. ↩

        2. @@ -688,15 +688,15 @@ Hi! 
          as well as code blocks
          -

          or, naturally, simple paragraphs. ↩

          +

          or, naturally, simple paragraphs. ↩

        3. -

          no code block here (spaces are stripped away) ↩

          +

          no code block here (spaces are stripped away) ↩

        4. this is now a code block (8 spaces indentation)
          -↩ +↩
        @@ -711,12 +711,12 @@ This footnote is referenced[^a-footnote] multiple times, in lots of different pl [^a-footnote]: This footnote definition should have three backrefs. . -

        This is some text. It has a footnote1.

        -

        This footnote is referenced1 multiple times, in lots of different places.1

        -
        +

        This is some text. It has a footnote1.

        +

        This footnote is referenced1 multiple times, in lots of different places.1

        +
        1. -

          This footnote definition should have three backrefs. ↩ ↩2 ↩3

          +

          This footnote definition should have three backrefs. ↩ ↩2 ↩3

        @@ -729,11 +729,11 @@ Hello[^">] [^">]: pwned . -

        Hello1

        -
        +

        Hello1

        +
        1. -

          pwned ↩

          +

          pwned ↩

        diff --git a/test/regression.txt b/test/regression.txt index e8e593f25..d4fbe3102 100644 --- a/test/regression.txt +++ b/test/regression.txt @@ -4,7 +4,8 @@ Issue #113: EOL character weirdness on Windows (Important: first line ends with CR + CR + LF) ```````````````````````````````` example -line1 +line1 + line2 .

        line1

        @@ -175,7 +176,7 @@ A footnote in a paragraph[^1] [^1]: a footnote . -

        A footnote in a paragraph1

        +

        A footnote in a paragraph1

        @@ -185,15 +186,15 @@ A footnote in a paragraph[^1] - +
        foot 1foot 1 note
        -
        +
        1. -

          a footnote ↩ ↩2

          +

          a footnote ↩ ↩2

        @@ -279,14 +280,14 @@ This is some text. It has a citation.[^citation] [^citation]: This is a long winded parapgraph that also has another citation.[^another-citation] . -

        This is some text. It has a citation.1

        -
        +

        This is some text. It has a citation.1

        +
        1. -

          This is a long winded parapgraph that also has another citation.2 ↩

          +

          This is a long winded parapgraph that also has another citation.2 ↩

        2. -

          My second citation. ↩

          +

          My second citation. ↩

        @@ -301,14 +302,14 @@ This is some text. It has two footnotes references, side-by-side without any spa [^footnote2]: Goodbye. . -

        This is some text. It has two footnotes references, side-by-side without any spaces,12 which are definitely not link references.

        -
        +

        This is some text. It has two footnotes references, side-by-side without any spaces,12 which are definitely not link references.

        +
        1. -

          Hello. ↩

          +

          Hello. ↩

        2. -

          Goodbye. ↩

          +

          Goodbye. ↩

        @@ -325,15 +326,15 @@ It has another footnote that contains many different characters (the autolinker [^widely-cited]: this renders properly. . -

        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.1

        -

        It has another footnote that contains many different characters (the autolinker was also breaking on _).2

        -
        +

        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.1

        +

        It has another footnote that contains many different characters (the autolinker was also breaking on _).2

        +
        1. -

          this renders properly. ↩

          +

          this renders properly. ↩

        2. -

          so does this. ↩

          +

          so does this. ↩

        From d86bddf0a69f1da1df3d5b5c6b474be0906ce332 Mon Sep 17 00:00:00 2001 From: Phill MV Date: Thu, 16 Sep 2021 12:44:27 -0400 Subject: [PATCH 34/36] Bump version to 0.29.0.gfm.2 --- CMakeLists.txt | 2 +- changelog.txt | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 34773819f..0c6e23ce5 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -4,7 +4,7 @@ project(cmark-gfm) set(PROJECT_VERSION_MAJOR 0) set(PROJECT_VERSION_MINOR 29) set(PROJECT_VERSION_PATCH 0) -set(PROJECT_VERSION_GFM 1) +set(PROJECT_VERSION_GFM 2) set(PROJECT_VERSION ${PROJECT_VERSION_MAJOR}.${PROJECT_VERSION_MINOR}.${PROJECT_VERSION_PATCH}.gfm.${PROJECT_VERSION_GFM}) include("FindAsan.cmake") diff --git a/changelog.txt b/changelog.txt index 5ba1e62b3..a8f7bb1b1 100644 --- a/changelog.txt +++ b/changelog.txt @@ -1,3 +1,13 @@ +[0.29.0.gfm.2] + * Fixed issues with footnote rendering when used with the autolinker (#121), + and when footnotes are adjacent (#139). + * We now allow footnotes to be referenced from inside a footnote definition, + we use the footnote label for the fnref href text when rendering html, and + we insert multiple backrefs when a footnote has been referenced multiple + times (#229, #230) + * We added new data- attributes to footnote html rendering to make them + easier to style (#234) + [0.29.0.gfm.1] * Fixed denial of service bug in GFM's table extension From ac80f7b56522ffa158e1f0c14a611ffccacd4027 Mon Sep 17 00:00:00 2001 From: Bas Alberts Date: Tue, 22 Feb 2022 14:38:03 -0500 Subject: [PATCH 35/36] prevent integer overflow in row_from_string * added explicit check for UINT16_MAX boundary on row->n_columns * added additional checks for row_from_string NULL returns to prevent NULL dereferences on error cases * added additional check to ensure n_columns between marker and header rows always match prior to any alignment processing * allocate alignment array based on marker rows rather than header rows * prevent memory leak on dangling node when encountering row_from_string error in try_opening_table_row * add explicit integer overflow error marker to not overload offset semantics in row_from_string with other implied error conditions --- extensions/table.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/extensions/table.c b/extensions/table.c index a5bb44067..b9bf48404 100644 --- a/extensions/table.c +++ b/extensions/table.c @@ -129,6 +129,7 @@ static table_row *row_from_string(cmark_syntax_extension *self, bufsize_t cell_matched = 1, pipe_matched = 1, offset; int expect_more_cells = 1; int row_end_offset = 0; + int int_overflow_abort = 0; row = (table_row *)parser->mem->calloc(1, sizeof(table_row)); row->n_columns = 0; @@ -161,6 +162,12 @@ static table_row *row_from_string(cmark_syntax_extension *self, ++cell->internal_offset; } + // make sure we never wrap row->n_columns + // offset will != len and our exit will clean up as intended + if (row->n_columns == UINT16_MAX) { + int_overflow_abort = 1; + break; + } row->n_columns += 1; row->cells = cmark_llist_append(parser->mem, row->cells, cell); } @@ -194,7 +201,7 @@ static table_row *row_from_string(cmark_syntax_extension *self, } } - if (offset != len || row->n_columns == 0) { + if (offset != len || row->n_columns == 0 || int_overflow_abort) { free_table_row(parser->mem, row); row = NULL; } @@ -241,6 +248,11 @@ static cmark_node *try_opening_table_header(cmark_syntax_extension *self, marker_row = row_from_string(self, parser, input + cmark_parser_get_first_nonspace(parser), len - cmark_parser_get_first_nonspace(parser)); + // assert may be optimized out, don't rely on it for security boundaries + if (!marker_row) { + return parent_container; + } + assert(marker_row); cmark_arena_push(); @@ -264,6 +276,12 @@ static cmark_node *try_opening_table_header(cmark_syntax_extension *self, len - cmark_parser_get_first_nonspace(parser)); header_row = row_from_string(self, parser, (unsigned char *)parent_string, (int)strlen(parent_string)); + // row_from_string can return NULL, add additional check to ensure n_columns match + if (!marker_row || !header_row || header_row->n_columns != marker_row->n_columns) { + free_table_row(parser->mem, marker_row); + free_table_row(parser->mem, header_row); + return parent_container; + } } if (!cmark_node_set_type(parent_container, CMARK_NODE_TABLE)) { @@ -281,8 +299,10 @@ static cmark_node *try_opening_table_header(cmark_syntax_extension *self, parent_container->as.opaque = parser->mem->calloc(1, sizeof(node_table)); set_n_table_columns(parent_container, header_row->n_columns); + // allocate alignments based on marker_row->n_columns + // since we populate the alignments array based on marker_row->cells uint8_t *alignments = - (uint8_t *)parser->mem->calloc(header_row->n_columns, sizeof(uint8_t)); + (uint8_t *)parser->mem->calloc(marker_row->n_columns, sizeof(uint8_t)); cmark_llist *it = marker_row->cells; for (i = 0; it; it = it->next, ++i) { node_cell *node = (node_cell *)it->data; @@ -351,6 +371,12 @@ static cmark_node *try_opening_table_row(cmark_syntax_extension *self, row = row_from_string(self, parser, input + cmark_parser_get_first_nonspace(parser), len - cmark_parser_get_first_nonspace(parser)); + if (!row) { + // clean up the dangling node + cmark_node_free(table_row_block); + return NULL; + } + { cmark_llist *tmp; int i, table_columns = get_n_table_columns(parent_container); From ff164f188bc1eb23391c85436ab418463e7a030f Mon Sep 17 00:00:00 2001 From: Phill MV Date: Thu, 24 Feb 2022 14:51:59 -0500 Subject: [PATCH 36/36] Bump version to `0.29.0.gfm.3` --- CMakeLists.txt | 2 +- changelog.txt | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0c6e23ce5..42be7e445 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -4,7 +4,7 @@ project(cmark-gfm) set(PROJECT_VERSION_MAJOR 0) set(PROJECT_VERSION_MINOR 29) set(PROJECT_VERSION_PATCH 0) -set(PROJECT_VERSION_GFM 2) +set(PROJECT_VERSION_GFM 3) set(PROJECT_VERSION ${PROJECT_VERSION_MAJOR}.${PROJECT_VERSION_MINOR}.${PROJECT_VERSION_PATCH}.gfm.${PROJECT_VERSION_GFM}) include("FindAsan.cmake") diff --git a/changelog.txt b/changelog.txt index a8f7bb1b1..8fad876b6 100644 --- a/changelog.txt +++ b/changelog.txt @@ -1,3 +1,6 @@ +[0.29.0.gfm.3] + * Fixed heap memory corruption vulnerabiliy via integer overflow per https://github.com/github/cmark-gfm/security/advisories/GHSA-mc3g-88wq-6f4x + [0.29.0.gfm.2] * Fixed issues with footnote rendering when used with the autolinker (#121), and when footnotes are adjacent (#139).