Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sync in changes made to cmark-gfm after 2021-08-01 #29

Merged
merged 51 commits into from
Mar 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
74631f8
Copied over the actions CI workflows that @jgm added in commonmark/cm…
phillmv Aug 3, 2021
7346f8d
Turns out we don't have a lint task in our Makefile (yet).
phillmv Aug 3, 2021
2b2ffaf
Let's drop the fuzzer for now, looks like we have to set something up…
phillmv Aug 3, 2021
4e0a81f
Make normalize test compatible with python 3.8
keith-packard Mar 9, 2020
4bab14a
Removed outdated build status badges.
phillmv Aug 12, 2021
4a7186e
Run ci build only on push, thanks.
phillmv Aug 12, 2021
71e27f2
Footnotes now support being nested, i.e. a footnote may reference ano…
phillmv Aug 19, 2021
1f026ef
Fix for footnotes being confused for link references.
phillmv Aug 19, 2021
bb117ff
Fix for when footnote reference labels get broken up into multiple cm…
phillmv Aug 19, 2021
bf76871
Fix footnote reference label text, and add multiple backrefs.
phillmv Aug 19, 2021
272c999
Fixed footnote extension test to handle new footnote reference link l…
phillmv Aug 19, 2021
2cb2f7c
Added test example that exercises a single footnote being referenced …
phillmv Aug 19, 2021
8ccdaa7
Converted regression test to expect new footnote ref link labels.
phillmv Aug 19, 2021
a0de7d8
Added regression test that exercises nested footnotes.
phillmv Aug 20, 2021
7fa2372
Added test that properly exercises footnotes whose reference labels c…
phillmv Aug 20, 2021
740b987
Added test that exercises whether footnotes are confused for link ref…
phillmv Aug 20, 2021
582eb8a
Merge branch 'footnotes-fix-confused-for-link-reference' into all-foo…
phillmv Aug 20, 2021
c464de3
Merge branch 'footnotes-fix-when-across-multiple-nodes' into all-foot…
phillmv Aug 20, 2021
1aabfa3
Merge branch 'footnotes-fix-fnref-label-and-backrefs' into all-footno…
phillmv Aug 20, 2021
7b5d45d
Adapted existing regression tests to conform to new footnote ref label.
phillmv Aug 20, 2021
fdd7851
Added Actions CI badge.
phillmv Aug 23, 2021
0277638
Removed redundant 'import cgi' from normalize.py
phillmv Aug 23, 2021
fcf8b73
Merge pull request #226 from github/add-ci
phillmv Aug 23, 2021
993e869
Merge branch 'master' into fix-footnotes-nested-linkrefs-autolinker
phillmv Aug 23, 2021
b790eca
Bumped version to 0.29.0.gfm.1
phillmv Aug 25, 2021
32ffc77
Renamed cmark_node->footnote.{ix,count} to {ref_ix,def_count} to make…
phillmv Sep 1, 2021
1717040
Added cmark_node.parent_footnote_def, removed usage of 'user_data', m…
phillmv Sep 1, 2021
984b5ea
Merge branch 'master' into fix-footnotes-plus-fix-fnref-label-and-bac…
phillmv Sep 1, 2021
32002ec
replaced strbuf_put with strbuf_puts
phillmv Sep 1, 2021
6e186b3
WIP: what if we only free the nodes after calling process_emphasis?
phillmv Sep 2, 2021
d3a819c
Fix & regression test for use-after-free introduced in bb117ffa7f0dcc…
phillmv Sep 2, 2021
a1d171a
Fix for use-after-free bug introduced in 71e27f25f11c9a34f0532dba4599…
phillmv Sep 2, 2021
98a2544
Merge branch 'fix-footnotes-nested-linkrefs-autolinker' into fix-foot…
phillmv Sep 14, 2021
0f98e8a
Merge pull request #231 from github/bump-version-to-0290gfm1
phillmv Sep 14, 2021
d43ae4b
By default, always escape footnote hrefs when emitting html.
phillmv Sep 14, 2021
a86bbc5
added extension test that verifies that footnote labels get href esca…
phillmv Sep 15, 2021
586a22d
literal->data is probably NULL terminated, but just in case let's che…
phillmv Sep 15, 2021
de6feae
Added check for underflows when duping footnote ref literal.
phillmv Sep 15, 2021
5790bf2
Merge branch 'fix-footnotes-nested-linkrefs-autolinker' into fix-foot…
phillmv Sep 15, 2021
4bf57ea
Swapped calloc argument order, so that we use the function appropriat…
phillmv Sep 15, 2021
8474289
Swapped : for - when emitting html footnote ref labels.
phillmv Sep 15, 2021
b6e462f
Add data attributes for footnotes
talum Sep 14, 2021
eb5b719
Merge pull request #229 from github/fix-footnotes-nested-linkrefs-aut…
phillmv Sep 16, 2021
d7e50f0
Merge pull request #230 from github/fix-footnotes-plus-fix-fnref-labe…
phillmv Sep 16, 2021
9eb8858
Merge pull request #234 from github/add-attributes-to-footnotes
phillmv Sep 16, 2021
d86bddf
Bump version to 0.29.0.gfm.2
phillmv Sep 16, 2021
766f161
Merge pull request #235 from github/bump-version-to-0290gfm2
phillmv Sep 16, 2021
ac80f7b
prevent integer overflow in row_from_string
anticomputer Feb 22, 2022
ff164f1
Bump version to `0.29.0.gfm.3`
phillmv Feb 24, 2022
cf7577d
Merge pull request from GHSA-mc3g-88wq-6f4x
phillmv Mar 3, 2022
131078b
Merge remote-tracking branch 'github/master' into QuietMisdreavus/syn…
QuietMisdreavus Mar 10, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 83 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
name: CI tests

on: [push, workflow_dispatch]

jobs:
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'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does that make sense on macOS

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If gcc is available on macOS in GitHub Actions (i assume so since this is an upstream change), i guess they want to make sure to test it. However, this doesn't affect our fork, since we don't use GitHub Actions.

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
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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 3)
set(PROJECT_VERSION ${PROJECT_VERSION_MAJOR}.${PROJECT_VERSION_MINOR}.${PROJECT_VERSION_PATCH}.gfm.${PROJECT_VERSION_GFM})

include("FindAsan.cmake")
Expand Down
18 changes: 18 additions & 0 deletions changelog.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,21 @@
[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).
* 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
per https://github.com/github/cmark-gfm/security/advisories/GHSA-7gc6-9qr5-hc85

[0.29.0]

* Update spec to 0.29.
Expand Down
15 changes: 13 additions & 2 deletions src/blocks.c
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,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);
}
}
Expand All @@ -511,6 +510,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;

char n[32];
snprintf(n, sizeof(n), "%d", footnote->ix);
cmark_chunk_free(parser->mem, &cur->as.literal);
Expand Down Expand Up @@ -541,13 +549,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);
}

Expand Down
18 changes: 14 additions & 4 deletions src/commonmark.c
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,13 @@ 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);

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);
renderer->mem->free(footnote_label);

LIT("]");
}
break;
Expand All @@ -497,9 +503,13 @@ 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 *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);
renderer->mem->free(footnote_label);

LIT("]:\n");

cmark_strbuf_puts(renderer->prefix, " ");
Expand Down
23 changes: 23 additions & 0 deletions src/footnotes.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
59 changes: 40 additions & 19 deletions src/html.c
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,30 @@ 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, "<a href=\"#fnref");
char n[32];
snprintf(n, sizeof(n), "%d", renderer->footnote_ix);
cmark_strbuf_puts(html, n);
cmark_strbuf_puts(html, "\" class=\"footnote-backref\">↩</a>");
cmark_strbuf_puts(html, "<a href=\"#fnref-");
houdini_escape_href(html, node->as.literal.data, node->as.literal.len);
cmark_strbuf_puts(html, "\" class=\"footnote-backref\" data-footnote-backref aria-label=\"Back to content\">↩</a>");

if (node->footnote.def_count > 1)
{
for(int i = 2; i <= node->footnote.def_count; i++) {
char n[32];
snprintf(n, sizeof(n), "%d", i);

cmark_strbuf_puts(html, " <a href=\"#fnref-");
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\" data-footnote-backref aria-label=\"Back to content\">↩<sup class=\"footnote-ref\">");
cmark_strbuf_puts(html, n);
cmark_strbuf_puts(html, "</sup></a>");
}
}

return true;
}
Expand Down Expand Up @@ -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, "</p>\n");
}
Expand Down Expand Up @@ -405,16 +419,15 @@ 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, "<section class=\"footnotes\">\n<ol>\n");
cmark_strbuf_puts(html, "<section class=\"footnotes\" data-footnotes>\n<ol>\n");
}
++renderer->footnote_ix;
cmark_strbuf_puts(html, "<li id=\"fn");
char n[32];
snprintf(n, sizeof(n), "%d", renderer->footnote_ix);
cmark_strbuf_puts(html, n);

cmark_strbuf_puts(html, "<li id=\"fn-");
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)) {
if (S_put_footnote_backref(renderer, html, node)) {
cmark_strbuf_putc(html, '\n');
}
cmark_strbuf_puts(html, "</li>\n");
Expand All @@ -423,12 +436,20 @@ static int S_render_node(cmark_html_renderer *renderer, cmark_node *node,

case CMARK_NODE_FOOTNOTE_REFERENCE:
if (entering) {
cmark_strbuf_puts(html, "<sup class=\"footnote-ref\"><a href=\"#fn");
cmark_strbuf_put(html, node->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, "\">");
cmark_strbuf_put(html, node->as.literal.data, node->as.literal.len);
cmark_strbuf_puts(html, "<sup class=\"footnote-ref\"><a href=\"#fn-");
houdini_escape_href(html, node->parent_footnote_def->as.literal.data, node->parent_footnote_def->as.literal.len);
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, n);
}

cmark_strbuf_puts(html, "\" data-footnote-ref>");
houdini_escape_href(html, node->as.literal.data, node->as.literal.len);
cmark_strbuf_puts(html, "</a></sup>");
}
break;
Expand Down
2 changes: 2 additions & 0 deletions src/include/footnotes.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions src/include/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,13 @@ struct cmark_node {

cmark_syntax_extension *extension;

union {
int ref_ix;
int def_count;
} footnote;

cmark_node *parent_footnote_def;

union {
cmark_chunk literal;
cmark_list list;
Expand Down
Loading