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

Revert bad fix for GHSA-66g8-4hjf-77xh #323

Merged
merged 5 commits into from
Apr 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 0 additions & 6 deletions extensions/table.c
Original file line number Diff line number Diff line change
Expand Up @@ -311,18 +311,12 @@ static cmark_node *try_opening_table_header(cmark_syntax_extension *self,
}
}

assert(cmark_node_get_type(parent_container) == CMARK_NODE_PARAGRAPH);
if (!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;
}

// Update the node counts after parent_container changed type.
assert(parent_container->next == NULL);
decr_open_block_count(parser, CMARK_NODE_PARAGRAPH);
incr_open_block_count(parser, CMARK_NODE_TABLE);

if (header_row->paragraph_offset) {
try_inserting_table_header_paragraph(parser, parent_container, (unsigned char *)parent_string,
header_row->paragraph_offset);
Expand Down
119 changes: 12 additions & 107 deletions src/blocks.c
Original file line number Diff line number Diff line change
Expand Up @@ -70,22 +70,6 @@ static void S_parser_feed(cmark_parser *parser, const unsigned char *buffer,
static void S_process_line(cmark_parser *parser, const unsigned char *buffer,
bufsize_t bytes);

static void subtract_open_block_counts(cmark_parser *parser, cmark_node *node) {
do {
decr_open_block_count(parser, S_type(node));
node->flags &= ~CMARK_NODE__OPEN_BLOCK;
node = node->last_child;
} while (node);
}

static void add_open_block_counts(cmark_parser *parser, cmark_node *node) {
do {
incr_open_block_count(parser, S_type(node));
node->flags |= CMARK_NODE__OPEN_BLOCK;
node = node->last_child;
} while (node);
}

static cmark_node *make_block(cmark_mem *mem, cmark_node_type tag,
int start_line, int start_column) {
cmark_node *e;
Expand Down Expand Up @@ -145,7 +129,6 @@ static void cmark_parser_reset(cmark_parser *parser) {
parser->refmap = cmark_reference_map_new(parser->mem);
parser->root = document;
parser->current = document;
add_open_block_counts(parser, document);

parser->syntax_extensions = saved_exts;
parser->inline_syntax_extensions = saved_inline_exts;
Expand Down Expand Up @@ -259,18 +242,15 @@ static void remove_trailing_blank_lines(cmark_strbuf *ln) {
// Check to see if a node ends with a blank line, descending
// if needed into lists and sublists.
static bool S_ends_with_blank_line(cmark_node *node) {
while (true) {
if (S_last_line_checked(node)) {
return(S_last_line_blank(node));
} else if ((S_type(node) == CMARK_NODE_LIST ||
S_type(node) == CMARK_NODE_ITEM) && node->last_child) {
S_set_last_line_checked(node);
node = node->last_child;
continue;
} else {
S_set_last_line_checked(node);
return (S_last_line_blank(node));
}
if (S_last_line_checked(node)) {
return(S_last_line_blank(node));
} else if ((S_type(node) == CMARK_NODE_LIST ||
S_type(node) == CMARK_NODE_ITEM) && node->last_child) {
S_set_last_line_checked(node);
return(S_ends_with_blank_line(node->last_child));
} else {
S_set_last_line_checked(node);
return (S_last_line_blank(node));
}
}

Expand Down Expand Up @@ -330,12 +310,6 @@ static cmark_node *finalize(cmark_parser *parser, cmark_node *b) {
has_content = resolve_reference_link_definitions(parser, b);
if (!has_content) {
// remove blank node (former reference def)
if (b->flags & CMARK_NODE__OPEN_BLOCK) {
decr_open_block_count(parser, S_type(b));
if (b->prev) {
add_open_block_counts(parser, b->prev);
}
}
cmark_node_free(b);
}
break;
Expand Down Expand Up @@ -408,17 +382,6 @@ static cmark_node *finalize(cmark_parser *parser, cmark_node *b) {
return parent;
}

// Recalculates the number of open blocks. Returns true if it matches what's currently stored
// in parser. (Used to check that the counts in parser, which are updated incrementally, are
// correct.)
bool check_open_block_counts(cmark_parser *parser) {
cmark_parser tmp_parser = {0}; // Only used for its open_block_counts and total_open_blocks fields.
add_open_block_counts(&tmp_parser, parser->root);
return
tmp_parser.total_open_blocks == parser->total_open_blocks &&
memcmp(tmp_parser.open_block_counts, parser->open_block_counts, sizeof(parser->open_block_counts)) == 0;
}

// Add a node as child of another. Return pointer to child.
static cmark_node *add_child(cmark_parser *parser, cmark_node *parent,
cmark_node_type block_type, int start_column) {
Expand All @@ -437,14 +400,11 @@ static cmark_node *add_child(cmark_parser *parser, cmark_node *parent,
if (parent->last_child) {
parent->last_child->next = child;
child->prev = parent->last_child;
subtract_open_block_counts(parser, parent->last_child);
} else {
parent->first_child = child;
child->prev = NULL;
}
parent->last_child = child;
add_open_block_counts(parser, child);

return child;
}

Expand Down Expand Up @@ -1087,14 +1047,8 @@ static cmark_node *check_open_blocks(cmark_parser *parser, cmark_chunk *input,
*all_matched = false;
cmark_node *container = parser->root;
cmark_node_type cont_type;
cmark_parser tmp_parser; // Only used for its open_block_counts and total_open_blocks fields.
memcpy(tmp_parser.open_block_counts, parser->open_block_counts, sizeof(parser->open_block_counts));
tmp_parser.total_open_blocks = parser->total_open_blocks;

assert(check_open_block_counts(parser));

while (S_last_child_is_open(container)) {
decr_open_block_count(&tmp_parser, S_type(container));
container = container->last_child;
cont_type = S_type(container);

Expand All @@ -1106,53 +1060,6 @@ static cmark_node *check_open_blocks(cmark_parser *parser, cmark_chunk *input,
continue;
}

// This block of code is a workaround for the quadratic performance
// issue described here (issue 2):
//
// https://github.com/github/cmark-gfm/security/advisories/GHSA-66g8-4hjf-77xh
//
// If the current line is empty then we might be able to skip directly
// to the end of the list of open blocks. To determine whether this is
// possible, we have been maintaining a count of the number of
// different types of open blocks. The main criterium is that every
// remaining block, except the last element of the list, is a LIST or
// ITEM. The code below checks the conditions, and if they're ok, skips
// forward to parser->current.
if (parser->blank && parser->indent == 0) { // Current line is empty
// Make sure that parser->current doesn't point to a closed block.
if (parser->current->flags & CMARK_NODE__OPEN_BLOCK) {
if (parser->current->flags & CMARK_NODE__OPEN) {
const size_t n_list = read_open_block_count(&tmp_parser, CMARK_NODE_LIST);
const size_t n_item = read_open_block_count(&tmp_parser, CMARK_NODE_ITEM);
// At most one block can be something other than a LIST or ITEM.
if (n_list + n_item + 1 >= tmp_parser.total_open_blocks) {
// Check that parser->current is suitable for jumping to.
switch (S_type(parser->current)) {
case CMARK_NODE_LIST:
case CMARK_NODE_ITEM:
if (n_list + n_item != tmp_parser.total_open_blocks) {
if (parser->current->last_child == NULL) {
// There's another node type somewhere in the middle of
// the list, so don't attempt the optimization.
break;
}
}
// fall through
case CMARK_NODE_CODE_BLOCK:
case CMARK_NODE_PARAGRAPH:
case CMARK_NODE_HTML_BLOCK:
// Jump to parser->current
container = parser->current;
cont_type = S_type(container);
break;
default:
break;
}
}
}
}
}

switch (cont_type) {
case CMARK_NODE_BLOCK_QUOTE:
if (!parse_block_quote_prefix(parser, input))
Expand Down Expand Up @@ -1286,9 +1193,8 @@ static void open_new_blocks(cmark_parser *parser, cmark_node **container,
has_content = resolve_reference_link_definitions(parser, *container);

if (has_content) {
cmark_node_set_type(*container, CMARK_NODE_HEADING);
decr_open_block_count(parser, CMARK_NODE_PARAGRAPH);
incr_open_block_count(parser, CMARK_NODE_HEADING);

(*container)->type = (uint16_t)CMARK_NODE_HEADING;
(*container)->as.heading.level = lev;
(*container)->as.heading.setext = true;
S_advance_offset(parser, input, input->len - 1 - parser->offset, false);
Expand Down Expand Up @@ -1443,7 +1349,7 @@ static void add_text_to_container(cmark_parser *parser, cmark_node *container,
S_set_last_line_blank(container, last_line_blank);

tmp = container;
while (tmp->parent && S_last_line_blank(tmp->parent)) {
while (tmp->parent) {
S_set_last_line_blank(tmp->parent, false);
tmp = tmp->parent;
}
Expand Down Expand Up @@ -1572,7 +1478,6 @@ static void S_process_line(cmark_parser *parser, const unsigned char *buffer,

parser->line_number++;

assert(parser->current->next == NULL);
last_matched_container = check_open_blocks(parser, &input, &all_matched);

if (!last_matched_container)
Expand Down
10 changes: 0 additions & 10 deletions src/cmark-gfm.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,6 @@ char *cmark_markdown_to_html(const char *text, size_t len, int options);
#define CMARK_NODE_TYPE_MASK (0xc000)
#define CMARK_NODE_VALUE_MASK (0x3fff)

/**
* This is the maximum number of block types (CMARK_NODE_DOCUMENT,
* CMARK_NODE_HEADING, ...). It needs to be bigger than the number of
* hardcoded block types (below) to allow for extensions (see
* cmark_syntax_extension_add_node). But it also determines the size of the
* open_block_counts array in the cmark_parser struct, so we don't want it
* to be excessively large.
*/
#define CMARK_NODE_TYPE_BLOCK_LIMIT 0x20

typedef enum {
/* Error status */
CMARK_NODE_NONE = 0x0000,
Expand Down
7 changes: 3 additions & 4 deletions src/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,12 @@ typedef struct {

enum cmark_node__internal_flags {
CMARK_NODE__OPEN = (1 << 0),
CMARK_NODE__OPEN_BLOCK = (1 << 1),
CMARK_NODE__LAST_LINE_BLANK = (1 << 2),
CMARK_NODE__LAST_LINE_CHECKED = (1 << 3),
CMARK_NODE__LAST_LINE_BLANK = (1 << 1),
CMARK_NODE__LAST_LINE_CHECKED = (1 << 2),

// Extensions can register custom flags by calling `cmark_register_node_flag`.
// This is the starting value for the custom flags.
CMARK_NODE__REGISTER_FIRST = (1 << 4),
CMARK_NODE__REGISTER_FIRST = (1 << 3),
};

typedef uint16_t cmark_node_internal_flags;
Expand Down
39 changes: 0 additions & 39 deletions src/parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,47 +50,8 @@ struct cmark_parser {
cmark_llist *syntax_extensions;
cmark_llist *inline_syntax_extensions;
cmark_ispunct_func backslash_ispunct;

/**
* The "open" blocks are the blocks visited by the loop in
* check_open_blocks (blocks.c). I.e. the blocks in this list:
*
* parser->root->last_child->...->last_child
*
* open_block_counts is used to keep track of how many of each type of
* node are currently in the open blocks list. Knowing these counts can
* sometimes help to end the loop in check_open_blocks early, improving
* efficiency.
*
* The count is stored at this offset: type - CMARK_NODE_TYPE_BLOCK - 1
* For example, CMARK_NODE_LIST (0x8003) is stored at offset 2.
*/
size_t open_block_counts[CMARK_NODE_TYPE_BLOCK_LIMIT];
size_t total_open_blocks;
};

static CMARK_INLINE void incr_open_block_count(cmark_parser *parser, cmark_node_type type) {
assert(type > CMARK_NODE_TYPE_BLOCK);
assert(type <= CMARK_NODE_TYPE_BLOCK + CMARK_NODE_TYPE_BLOCK_LIMIT);
parser->open_block_counts[type - CMARK_NODE_TYPE_BLOCK - 1]++;
parser->total_open_blocks++;
}

static CMARK_INLINE void decr_open_block_count(cmark_parser *parser, cmark_node_type type) {
assert(type > CMARK_NODE_TYPE_BLOCK);
assert(type <= CMARK_NODE_TYPE_BLOCK + CMARK_NODE_TYPE_BLOCK_LIMIT);
assert(parser->open_block_counts[type - CMARK_NODE_TYPE_BLOCK - 1] > 0);
parser->open_block_counts[type - CMARK_NODE_TYPE_BLOCK - 1]--;
assert(parser->total_open_blocks > 0);
parser->total_open_blocks--;
}

static CMARK_INLINE size_t read_open_block_count(cmark_parser *parser, cmark_node_type type) {
assert(type > CMARK_NODE_TYPE_BLOCK);
assert(type <= CMARK_NODE_TYPE_BLOCK + CMARK_NODE_TYPE_BLOCK_LIMIT);
return parser->open_block_counts[type - CMARK_NODE_TYPE_BLOCK - 1];
}

#ifdef __cplusplus
}
#endif
Expand Down
5 changes: 1 addition & 4 deletions src/syntax_extension.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,7 @@ cmark_syntax_extension *cmark_syntax_extension_new(const char *name) {
cmark_node_type cmark_syntax_extension_add_node(int is_inline) {
cmark_node_type *ref = !is_inline ? &CMARK_NODE_LAST_BLOCK : &CMARK_NODE_LAST_INLINE;

if ((*ref & CMARK_NODE_VALUE_MASK) >= CMARK_NODE_TYPE_BLOCK_LIMIT) {
// This assertion will fail if you try to register more extensions than
// are currently allowed by CMARK_NODE_TYPE_BLOCK_MAXNUM. Try increasing
// the limit.
if ((*ref & CMARK_NODE_VALUE_MASK) == CMARK_NODE_VALUE_MASK) {
assert(false);
return (cmark_node_type) 0;
}
Expand Down