From b1687e6af1367c596ab75428b03af55666a66530 Mon Sep 17 00:00:00 2001 From: Bas Alberts Date: Tue, 22 Feb 2022 14:46:11 -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 | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/extensions/table.c b/extensions/table.c index 9829c3d1a..c19a3a955 100644 --- a/extensions/table.c +++ b/extensions/table.c @@ -115,6 +115,7 @@ static table_row *row_from_string(cmark_syntax_extension *self, int len) { table_row *row = NULL; bufsize_t cell_matched = 1, pipe_matched = 1, offset; + int int_overflow_abort = 0; row = (table_row *)parser->mem->calloc(1, sizeof(table_row)); row->n_columns = 0; @@ -141,6 +142,12 @@ static table_row *row_from_string(cmark_syntax_extension *self, --cell->start_offset; ++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); } @@ -153,7 +160,7 @@ static table_row *row_from_string(cmark_syntax_extension *self, } } - if (offset != len || !row->n_columns) { + if (offset != len || !row->n_columns || int_overflow_abort) { free_table_row(parser->mem, row); row = NULL; } @@ -194,6 +201,15 @@ static cmark_node *try_opening_table_header(cmark_syntax_extension *self, input + cmark_parser_get_first_nonspace(parser), len - cmark_parser_get_first_nonspace(parser)); + if (!marker_row) { + // these calls are essentially moot + // but matching error handling style of project + free_table_row(parser->mem, marker_row); + cmark_arena_pop(); + return parent_container; + } + + // assert may be optimized out, don't rely on it for security boundaries assert(marker_row); if (header_row->n_columns != marker_row->n_columns) { @@ -211,7 +227,8 @@ static cmark_node *try_opening_table_header(cmark_syntax_extension *self, len - cmark_parser_get_first_nonspace(parser)); } - if (!cmark_node_set_type(parent_container, CMARK_NODE_TABLE)) { + // 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 || !cmark_node_set_type(parent_container, CMARK_NODE_TABLE)) { free_table_row(parser->mem, header_row); free_table_row(parser->mem, marker_row); return parent_container; @@ -223,8 +240,10 @@ static cmark_node *try_opening_table_header(cmark_syntax_extension *self, 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; @@ -293,6 +312,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 6cad5de581f9c12413f6a80cd9a42097656b5b7c Mon Sep 17 00:00:00 2001 From: Phill MV Date: Thu, 24 Feb 2022 14:54:12 -0500 Subject: [PATCH 2/2] Bump version to `0.28.3.gfm.21` --- CMakeLists.txt | 2 +- changelog.txt | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8ce839326..eb56f9319 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -19,7 +19,7 @@ set(PROJECT_NAME "cmark-gfm") set(PROJECT_VERSION_MAJOR 0) set(PROJECT_VERSION_MINOR 28) set(PROJECT_VERSION_PATCH 3) -set(PROJECT_VERSION_GFM 20) +set(PROJECT_VERSION_GFM 21) set(PROJECT_VERSION ${PROJECT_VERSION_MAJOR}.${PROJECT_VERSION_MINOR}.${PROJECT_VERSION_PATCH}.gfm.${PROJECT_VERSION_GFM} ) option(CMARK_TESTS "Build cmark-gfm tests and enable testing" ON) diff --git a/changelog.txt b/changelog.txt index 13732dd88..3b65588ca 100644 --- a/changelog.txt +++ b/changelog.txt @@ -1,3 +1,6 @@ +[0.28.3.gfm.21] + * Fixed heap memory corruption vulnerabiliy via integer overflow per https://github.com/github/cmark-gfm/security/advisories/GHSA-mc3g-88wq-6f4x + [0.28.3.gfm.20] * Add tasklist extension implementation (Watson1978, #94).