From ac80f7b56522ffa158e1f0c14a611ffccacd4027 Mon Sep 17 00:00:00 2001 From: Bas Alberts Date: Tue, 22 Feb 2022 14:38:03 -0500 Subject: [PATCH 1/2] 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 2/2] 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).