Skip to content

Commit

Permalink
Check null stack to prevent heap-buffer-overflow
Browse files Browse the repository at this point in the history
This patch adds a new macro STACK_NULL to check if given stack was initialized,
in order to fix #298, which is CVE-2024-35329.

The root cause is stack(document->nodes) was used before initialized, so check stack
before push.

According to the poc in [1], building it with
`gcc poc.c -o poc -lyaml -fsanitize=address`

Before this patch, the output is:
[root@test yaml-0.2.5]# ./poc
heap-buffer-overflow on libyaml/src/api.c:1274:10

=================================================================
==3867981==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 64 byte(s) in 1 object(s) allocated from:
    #0 0x7f571f6af1a7 in __interceptor_malloc (/usr/lib64/libasan.so.6+0xaf1a7)
    #1 0x7f5720127ac9 in yaml_document_add_sequence /root/libxml/yaml-0.2.5/src/api.c:1271

Direct leak of 22 byte(s) in 1 object(s) allocated from:
    #0 0x7f571f659707 in strdup (/usr/lib64/libasan.so.6+0x59707)
    #1 0x7f5720127ab7 in yaml_document_add_sequence /root/libxml/yaml-0.2.5/src/api.c:1268

Direct leak of 1 byte(s) in 1 object(s) allocated from:
    #0 0x7f571f6af1a7 in __interceptor_malloc (/usr/lib64/libasan.so.6+0xaf1a7)
    #1 0x7f5720125762 in yaml_stack_extend /root/libxml/yaml-0.2.5/src/api.c:126

SUMMARY: AddressSanitizer: 87 byte(s) leaked in 3 allocation(s).

After this patch, there are no memory leaks warnnings.

[1] https://drive.google.com/file/d/1xgQ9hJ7Sn5RVEsdMGvIy0s3b_bg3Wyk-/view?usp=sharing

Signed-off-by: Zhao Mengmeng <[email protected]>
  • Loading branch information
Zhao Mengmeng committed Jun 13, 2024
1 parent 840b65c commit 1c2f6b7
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/api.c
Original file line number Diff line number Diff line change
Expand Up @@ -1206,6 +1206,7 @@ yaml_document_add_scalar(yaml_document_t *document,

assert(document); /* Non-NULL document object is expected. */
assert(value); /* Non-NULL value is expected. */
if (STACK_NULL(&context, document->nodes)) goto error;

if (!tag) {
tag = (yaml_char_t *)YAML_DEFAULT_SCALAR_TAG;
Expand Down Expand Up @@ -1258,6 +1259,7 @@ yaml_document_add_sequence(yaml_document_t *document,
yaml_node_t node;

assert(document); /* Non-NULL document object is expected. */
if (STACK_NULL(&context, document->nodes)) goto error;

if (!tag) {
tag = (yaml_char_t *)YAML_DEFAULT_SEQUENCE_TAG;
Expand Down Expand Up @@ -1303,6 +1305,7 @@ yaml_document_add_mapping(yaml_document_t *document,
yaml_node_t node;

assert(document); /* Non-NULL document object is expected. */
if (STACK_NULL(&context, document->nodes)) goto error;

if (!tag) {
tag = (yaml_char_t *)YAML_DEFAULT_MAPPING_TAG;
Expand Down
5 changes: 5 additions & 0 deletions src/yaml_private.h
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,11 @@ yaml_stack_extend(void **start, void **top, void **end);
YAML_DECLARE(int)
yaml_queue_extend(void **start, void **head, void **tail, void **end);

#define STACK_NULL(context,stack) \
(((stack).start != NULL) ? 0 : \
((context)->error = YAML_MEMORY_ERROR, \
1))

#define STACK_INIT(context,stack,type) \
(((stack).start = (type)yaml_malloc(INITIAL_STACK_SIZE*sizeof(*(stack).start))) ? \
((stack).top = (stack).start, \
Expand Down

0 comments on commit 1c2f6b7

Please sign in to comment.