From 1d17fa9d5af3215b9c969c66aa2fe22a1030b8a1 Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Fri, 26 May 2023 11:48:13 +0100 Subject: [PATCH 1/7] Fix GHSL-2023-119: prevent quadratic performance by not allowing very deeply nested footnote definitions. --- src/blocks.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/blocks.c b/src/blocks.c index 03a58748b..a1988880a 100644 --- a/src/blocks.c +++ b/src/blocks.c @@ -1217,7 +1217,8 @@ static void open_new_blocks(cmark_parser *parser, cmark_node **container, parser->first_nonspace + 1); S_advance_offset(parser, input, input->len - 1 - parser->offset, false); } else if (!indented && - parser->options & CMARK_OPT_FOOTNOTES && + (parser->options & CMARK_OPT_FOOTNOTES) && + depth < MAX_LIST_DEPTH && (matched = scan_footnote_definition(input, parser->first_nonspace))) { cmark_chunk c = cmark_chunk_dup(input, parser->first_nonspace + 2, matched - 2); cmark_chunk_to_cstr(parser->mem, &c); From 5e8ad61d0a79eb7f7b8ae0863e2ee19387f734f0 Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Thu, 8 Jun 2023 22:41:25 +0100 Subject: [PATCH 2/7] Fix GHSL-2023-117: store cell index on node so that it doesn't need to be recomputed during rendering. --- extensions/table.c | 38 ++++++++++++++++++++++---------------- src/node.h | 1 + 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/extensions/table.c b/extensions/table.c index e53ea3154..339fe01bc 100644 --- a/extensions/table.c +++ b/extensions/table.c @@ -98,6 +98,23 @@ static int set_table_alignments(cmark_node *node, uint8_t *alignments) { return 1; } +static uint8_t get_cell_alignment(cmark_node *node) { + if (!node || node->type != CMARK_NODE_TABLE_CELL) + return 0; + + const uint8_t *alignments = get_table_alignments(node->parent->parent); + int i = node->as.custom_int; + return alignments[i]; +} + +static int set_cell_index(cmark_node *node, int i) { + if (!node || node->type != CMARK_NODE_TABLE_CELL) + return 0; + + node->as.custom_int = i; + return 1; +} + static cmark_strbuf *unescape_pipes(cmark_mem *mem, unsigned char *string, bufsize_t len) { cmark_strbuf *res = (cmark_strbuf *)mem->calloc(1, sizeof(cmark_strbuf)); @@ -333,7 +350,6 @@ static cmark_node *try_opening_table_header(cmark_syntax_extension *self, for (i = 0; i < marker_row->n_columns; ++i) { node_cell *node = &marker_row->cells[i]; bool left = node->buf->ptr[0] == ':', right = node->buf->ptr[node->buf->size - 1] == ':'; - if (left && right) alignments[i] = 'c'; else if (left) @@ -363,6 +379,7 @@ static cmark_node *try_opening_table_header(cmark_syntax_extension *self, header_cell->end_column = parent_container->start_column + cell->end_offset; cmark_node_set_string_content(header_cell, (char *) cell->buf->ptr); cmark_node_set_syntax_extension(header_cell, self); + set_cell_index(header_cell, i); } } @@ -412,12 +429,14 @@ static cmark_node *try_opening_table_row(cmark_syntax_extension *self, node->end_column = parent_container->start_column + cell->end_offset; cmark_node_set_string_content(node, (char *) cell->buf->ptr); cmark_node_set_syntax_extension(node, self); + set_cell_index(node, i); } for (; i < table_columns; ++i) { cmark_node *node = cmark_parser_add_child( parser, table_row_block, CMARK_NODE_TABLE_CELL, 0); cmark_node_set_syntax_extension(node, self); + set_cell_index(node, i); } } @@ -602,13 +621,7 @@ static const char *xml_attr(cmark_syntax_extension *extension, cmark_node *node) { if (node->type == CMARK_NODE_TABLE_CELL) { if (cmark_gfm_extensions_get_table_row_is_header(node->parent)) { - uint8_t *alignments = get_table_alignments(node->parent->parent); - int i = 0; - cmark_node *n; - for (n = node->parent->first_child; n; n = n->next, ++i) - if (n == node) - break; - switch (alignments[i]) { + switch (get_cell_alignment(node)) { case 'l': return " align=\"left\""; case 'c': return " align=\"center\""; case 'r': return " align=\"right\""; @@ -696,7 +709,6 @@ static void html_render(cmark_syntax_extension *extension, cmark_event_type ev_type, int options) { bool entering = (ev_type == CMARK_EVENT_ENTER); cmark_strbuf *html = renderer->html; - cmark_node *n; // XXX: we just monopolise renderer->opaque. struct html_table_state *table_state = @@ -745,7 +757,6 @@ static void html_render(cmark_syntax_extension *extension, } } } else if (node->type == CMARK_NODE_TABLE_CELL) { - uint8_t *alignments = get_table_alignments(node->parent->parent); if (entering) { cmark_html_render_cr(html); if (table_state->in_table_header) { @@ -754,12 +765,7 @@ static void html_render(cmark_syntax_extension *extension, cmark_strbuf_puts(html, "parent->first_child; n; n = n->next, ++i) - if (n == node) - break; - - switch (alignments[i]) { + switch (get_cell_alignment(node)) { case 'l': html_table_add_align(html, "left", options); break; case 'c': html_table_add_align(html, "center", options); break; case 'r': html_table_add_align(html, "right", options); break; diff --git a/src/node.h b/src/node.h index 38ac4a6fe..7264ff01e 100644 --- a/src/node.h +++ b/src/node.h @@ -105,6 +105,7 @@ struct cmark_node { cmark_link link; cmark_custom custom; int html_block_type; + int custom_int; // For extensions to store an int void *opaque; } as; }; From 2c5212e0508bbf19e9c7ec9de366b792cbdd6556 Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Thu, 8 Jun 2023 22:44:00 +0100 Subject: [PATCH 3/7] Fix GHSL-2023-118: limit number of autocompleted table cells to prevent DOS. --- extensions/table.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/extensions/table.c b/extensions/table.c index 339fe01bc..06d67cdc3 100644 --- a/extensions/table.c +++ b/extensions/table.c @@ -11,6 +11,9 @@ #include "table.h" #include "cmark-gfm-core-extensions.h" +// Limit to prevent a malicious input from causing a denial of service. +#define MAX_AUTOCOMPLETED_CELLS 0x80000 + // Custom node flag, initialized in `create_table_extension`. static cmark_node_internal_flags CMARK_NODE__TABLE_VISITED; @@ -31,6 +34,8 @@ typedef struct { typedef struct { uint16_t n_columns; uint8_t *alignments; + int n_rows; + int n_nonempty_cells; } node_table; typedef struct { @@ -83,6 +88,33 @@ static int set_n_table_columns(cmark_node *node, uint16_t n_columns) { return 1; } +// Increment the number of rows in the table. Also update n_nonempty_cells, +// which keeps track of the number of cells which were parsed from the +// input file. (If one of the rows is too short, then the trailing cells +// are autocompleted. Autocompleted cells are not counted in n_nonempty_cells.) +// The purpose of this is to prevent a malicious input from generating a very +// large number of autocompleted cells, which could cause a denial of service +// vulnerability. +static int incr_table_row_count(cmark_node *node, int i) { + if (!node || node->type != CMARK_NODE_TABLE) { + return 0; + } + + ((node_table *)node->as.opaque)->n_rows++; + ((node_table *)node->as.opaque)->n_nonempty_cells += i; + return 1; +} + +// Calculate the number of autocompleted cells. +static int get_n_autocompleted_cells(cmark_node *node) { + if (!node || node->type != CMARK_NODE_TABLE) { + return 0; + } + + const node_table *nt = (node_table *)node->as.opaque; + return (nt->n_columns * nt->n_rows) - nt->n_nonempty_cells; +} + static uint8_t *get_table_alignments(cmark_node *node) { if (!node || node->type != CMARK_NODE_TABLE) return 0; @@ -383,6 +415,8 @@ static cmark_node *try_opening_table_header(cmark_syntax_extension *self, } } + incr_table_row_count(parent_container, i); + cmark_parser_advance_offset( parser, (char *)input, (int)strlen((char *)input) - 1 - cmark_parser_get_offset(parser), false); @@ -402,6 +436,10 @@ static cmark_node *try_opening_table_row(cmark_syntax_extension *self, if (cmark_parser_is_blank(parser)) return NULL; + if (get_n_autocompleted_cells(parent_container) > MAX_AUTOCOMPLETED_CELLS) { + return NULL; + } + table_row_block = cmark_parser_add_child(parser, parent_container, CMARK_NODE_TABLE_ROW, parent_container->start_column); @@ -432,6 +470,8 @@ static cmark_node *try_opening_table_row(cmark_syntax_extension *self, set_cell_index(node, i); } + incr_table_row_count(parent_container, i); + for (; i < table_columns; ++i) { cmark_node *node = cmark_parser_add_child( parser, table_row_block, CMARK_NODE_TABLE_CELL, 0); From 94f38eb1e43e698c6912ea1ecf59f2c452d63d62 Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Tue, 20 Jun 2023 12:16:49 -0400 Subject: [PATCH 4/7] Update src/node.h Co-authored-by: Phill MV --- src/node.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node.h b/src/node.h index 7264ff01e..73ca76053 100644 --- a/src/node.h +++ b/src/node.h @@ -105,7 +105,7 @@ struct cmark_node { cmark_link link; cmark_custom custom; int html_block_type; - int custom_int; // For extensions to store an int + int cell_index; // For keeping track of TABLE_CELL table alignments void *opaque; } as; }; From d4a5cc1397b546f1bd0e602a930a13d5852a2bd3 Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Tue, 20 Jun 2023 17:23:23 +0100 Subject: [PATCH 5/7] Rename custom_int -> cell_index. --- extensions/table.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/table.c b/extensions/table.c index 06d67cdc3..fa023a3f0 100644 --- a/extensions/table.c +++ b/extensions/table.c @@ -135,7 +135,7 @@ static uint8_t get_cell_alignment(cmark_node *node) { return 0; const uint8_t *alignments = get_table_alignments(node->parent->parent); - int i = node->as.custom_int; + int i = node->as.cell_index; return alignments[i]; } @@ -143,7 +143,7 @@ static int set_cell_index(cmark_node *node, int i) { if (!node || node->type != CMARK_NODE_TABLE_CELL) return 0; - node->as.custom_int = i; + node->as.cell_index = i; return 1; } From ed8f2fec2cec49f9f36cc6409836a2840b488b27 Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Tue, 20 Jun 2023 17:28:28 +0100 Subject: [PATCH 6/7] Add newline --- extensions/table.c | 1 + 1 file changed, 1 insertion(+) diff --git a/extensions/table.c b/extensions/table.c index fa023a3f0..a54cbe808 100644 --- a/extensions/table.c +++ b/extensions/table.c @@ -382,6 +382,7 @@ static cmark_node *try_opening_table_header(cmark_syntax_extension *self, for (i = 0; i < marker_row->n_columns; ++i) { node_cell *node = &marker_row->cells[i]; bool left = node->buf->ptr[0] == ':', right = node->buf->ptr[node->buf->size - 1] == ':'; + if (left && right) alignments[i] = 'c'; else if (left) From 580f02178b367bf6dabbdfd565a14d1a83cdc5ba Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Tue, 20 Jun 2023 17:30:08 +0100 Subject: [PATCH 7/7] Remove unnecessary scope. --- extensions/table.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/extensions/table.c b/extensions/table.c index a54cbe808..cc106040e 100644 --- a/extensions/table.c +++ b/extensions/table.c @@ -402,18 +402,16 @@ static cmark_node *try_opening_table_header(cmark_syntax_extension *self, table_header->as.opaque = ntr = (node_table_row *)parser->mem->calloc(1, sizeof(node_table_row)); ntr->is_header = true; - { - for (i = 0; i < header_row->n_columns; ++i) { - node_cell *cell = &header_row->cells[i]; - cmark_node *header_cell = cmark_parser_add_child(parser, table_header, - CMARK_NODE_TABLE_CELL, parent_container->start_column + cell->start_offset); - header_cell->start_line = header_cell->end_line = parent_container->start_line; - header_cell->internal_offset = cell->internal_offset; - header_cell->end_column = parent_container->start_column + cell->end_offset; - cmark_node_set_string_content(header_cell, (char *) cell->buf->ptr); - cmark_node_set_syntax_extension(header_cell, self); - set_cell_index(header_cell, i); - } + for (i = 0; i < header_row->n_columns; ++i) { + node_cell *cell = &header_row->cells[i]; + cmark_node *header_cell = cmark_parser_add_child(parser, table_header, + CMARK_NODE_TABLE_CELL, parent_container->start_column + cell->start_offset); + header_cell->start_line = header_cell->end_line = parent_container->start_line; + header_cell->internal_offset = cell->internal_offset; + header_cell->end_column = parent_container->start_column + cell->end_offset; + cmark_node_set_string_content(header_cell, (char *) cell->buf->ptr); + cmark_node_set_syntax_extension(header_cell, self); + set_cell_index(header_cell, i); } incr_table_row_count(parent_container, i);