From 1c2f6b7f593927a48d2e3e73af2e9b652d8f0471 Mon Sep 17 00:00:00 2001 From: Zhao Mengmeng Date: Wed, 12 Jun 2024 18:17:35 +0800 Subject: [PATCH] Check null stack to prevent heap-buffer-overflow This patch adds a new macro STACK_NULL to check if given stack was initialized, in order to fix yaml#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 --- src/api.c | 3 +++ src/yaml_private.h | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/src/api.c b/src/api.c index 16f88bd7..3f3e29c1 100644 --- a/src/api.c +++ b/src/api.c @@ -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; @@ -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; @@ -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; diff --git a/src/yaml_private.h b/src/yaml_private.h index b3351c41..5a3ba184 100644 --- a/src/yaml_private.h +++ b/src/yaml_private.h @@ -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, \