Skip to content

Commit d36a2d9

Browse files
committed
node: prevent integer truncation in cmark_set_cstr
Refactor cmark_set_cstr to return success/failure status instead of aborting when input is too large, and let callers propagate the status. Define BUFSIZE_MAX in buffer.h and use it in buffer.c and node.c. Document custom allocator contract in cmark.h.
1 parent b320f40 commit d36a2d9

4 files changed

Lines changed: 32 additions & 24 deletions

File tree

src/buffer.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,10 @@ void cmark_strbuf_grow(cmark_strbuf *buf, bufsize_t target_size) {
4040
if (target_size < buf->asize)
4141
return;
4242

43-
if (target_size > (bufsize_t)(INT32_MAX / 2)) {
43+
if (target_size > (bufsize_t)(BUFSIZE_MAX / 2)) {
4444
fprintf(stderr,
4545
"[cmark] cmark_strbuf_grow requests buffer with size > %d, aborting\n",
46-
(INT32_MAX / 2));
46+
(BUFSIZE_MAX / 2));
4747
abort();
4848
}
4949

src/buffer.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ extern "C" {
1414
#endif
1515

1616
typedef int32_t bufsize_t;
17+
#define BUFSIZE_MAX INT32_MAX
1718

1819
typedef struct {
1920
cmark_mem *mem;

src/cmark.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,9 @@ typedef struct cmark_iter cmark_iter;
9393
*/
9494

9595
/** Defines the memory allocation functions to be used by CMark
96-
* when parsing and allocating a document tree
96+
* when parsing and allocating a document tree. Allocation functions
97+
* must not return NULL; if they are unable to satisfy a request,
98+
* they must abort the program (e.g. by calling abort()).
9799
*/
98100
typedef struct cmark_mem {
99101
void *(*calloc)(size_t, size_t);

src/node.c

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include <stdbool.h>
2+
#include <stdio.h>
23
#include <stdlib.h>
34
#include <string.h>
45

@@ -264,24 +265,34 @@ cmark_node *cmark_node_last_child(cmark_node *node) {
264265
}
265266
}
266267

267-
static bufsize_t cmark_set_cstr(cmark_mem *mem, unsigned char **dst,
268-
const char *src) {
268+
static int cmark_set_cstr(cmark_mem *mem, unsigned char **dst,
269+
bufsize_t *len, const char *src) {
269270
unsigned char *old = *dst;
270-
bufsize_t len;
271+
unsigned char *new_str = NULL;
272+
bufsize_t new_len = 0;
271273

272274
if (src && src[0]) {
273-
len = (bufsize_t)strlen(src);
274-
*dst = (unsigned char *)mem->realloc(NULL, len + 1);
275-
memcpy(*dst, src, len + 1);
275+
size_t srclen = strlen(src);
276+
if (srclen > (size_t)BUFSIZE_MAX - 1) {
277+
return 0;
278+
}
279+
new_len = (bufsize_t)srclen;
280+
new_str = (unsigned char *)mem->realloc(NULL, (size_t)new_len + 1);
281+
memcpy(new_str, src, (size_t)new_len + 1);
276282
} else {
277-
len = 0;
278-
*dst = NULL;
283+
new_len = 0;
284+
new_str = NULL;
285+
}
286+
287+
*dst = new_str;
288+
if (len) {
289+
*len = new_len;
279290
}
280291
if (old) {
281292
mem->free(old);
282293
}
283294

284-
return len;
295+
return 1;
285296
}
286297

287298
void *cmark_node_get_user_data(cmark_node *node) {
@@ -331,8 +342,7 @@ int cmark_node_set_literal(cmark_node *node, const char *content) {
331342
case CMARK_NODE_HTML_INLINE:
332343
case CMARK_NODE_CODE:
333344
case CMARK_NODE_CODE_BLOCK:
334-
node->len = cmark_set_cstr(node->mem, &node->data, content);
335-
return 1;
345+
return cmark_set_cstr(node->mem, &node->data, &node->len, content);
336346

337347
default:
338348
break;
@@ -500,8 +510,7 @@ int cmark_node_set_fence_info(cmark_node *node, const char *info) {
500510
}
501511

502512
if (node->type == CMARK_NODE_CODE_BLOCK) {
503-
cmark_set_cstr(node->mem, &node->as.code.info, info);
504-
return 1;
513+
return cmark_set_cstr(node->mem, &node->as.code.info, NULL, info);
505514
} else {
506515
return 0;
507516
}
@@ -531,8 +540,7 @@ int cmark_node_set_url(cmark_node *node, const char *url) {
531540
switch (node->type) {
532541
case CMARK_NODE_LINK:
533542
case CMARK_NODE_IMAGE:
534-
cmark_set_cstr(node->mem, &node->as.link.url, url);
535-
return 1;
543+
return cmark_set_cstr(node->mem, &node->as.link.url, NULL, url);
536544
default:
537545
break;
538546
}
@@ -564,8 +572,7 @@ int cmark_node_set_title(cmark_node *node, const char *title) {
564572
switch (node->type) {
565573
case CMARK_NODE_LINK:
566574
case CMARK_NODE_IMAGE:
567-
cmark_set_cstr(node->mem, &node->as.link.title, title);
568-
return 1;
575+
return cmark_set_cstr(node->mem, &node->as.link.title, NULL, title);
569576
default:
570577
break;
571578
}
@@ -597,8 +604,7 @@ int cmark_node_set_on_enter(cmark_node *node, const char *on_enter) {
597604
switch (node->type) {
598605
case CMARK_NODE_CUSTOM_INLINE:
599606
case CMARK_NODE_CUSTOM_BLOCK:
600-
cmark_set_cstr(node->mem, &node->as.custom.on_enter, on_enter);
601-
return 1;
607+
return cmark_set_cstr(node->mem, &node->as.custom.on_enter, NULL, on_enter);
602608
default:
603609
break;
604610
}
@@ -630,8 +636,7 @@ int cmark_node_set_on_exit(cmark_node *node, const char *on_exit) {
630636
switch (node->type) {
631637
case CMARK_NODE_CUSTOM_INLINE:
632638
case CMARK_NODE_CUSTOM_BLOCK:
633-
cmark_set_cstr(node->mem, &node->as.custom.on_exit, on_exit);
634-
return 1;
639+
return cmark_set_cstr(node->mem, &node->as.custom.on_exit, NULL, on_exit);
635640
default:
636641
break;
637642
}

0 commit comments

Comments
 (0)