From 40010798b32e0caadc9c52188d395d3e8b7c7a38 Mon Sep 17 00:00:00 2001 From: rootvector2 Date: Sun, 12 Apr 2026 22:48:50 +0530 Subject: [PATCH 1/2] Harden mutable-to-immutable copy against overflow, malformed structures, and deep recursion --- src/yyjson.c | 130 +++++++++++++++++++++++++++++++++------ test/test_json_mut_val.c | 50 +++++++++++++++ 2 files changed, 160 insertions(+), 20 deletions(-) diff --git a/src/yyjson.c b/src/yyjson.c index 6fd6f72..5cfd0bc 100644 --- a/src/yyjson.c +++ b/src/yyjson.c @@ -521,6 +521,12 @@ uint32_t yyjson_version(void) { /* maximum length of a number in incremental parsing */ #define INCR_NUM_MAX_LEN 1024 +/* + Maximum recursion depth for mutable value traversal and copy. + This prevents stack overflow when processing deeply nested or malicious inputs. + */ +#define YYJSON_MAX_DEPTH 1024 + /*============================================================================== @@ -2137,6 +2143,11 @@ static_inline bool size_add_is_overflow(usize size, usize add) { return size > (size + add); } +/** Returns whether the size is overflow after multiplication. */ +static_inline bool size_mul_is_overflow(usize size, usize mul) { + return mul != 0 && size > USIZE_MAX / mul; +} + /** Returns whether the size is power of 2 (size should not be 0). */ static_inline bool size_is_pow2(usize size) { return (size & (size - 1)) == 0; @@ -2804,62 +2815,127 @@ yyjson_mut_val *yyjson_mut_val_mut_copy(yyjson_mut_doc *doc, return NULL; } -/* Count the number of values and the total length of the strings. */ -static void yyjson_mut_stat(const yyjson_mut_val *val, - usize *val_sum, usize *str_sum) { - yyjson_type type = unsafe_yyjson_get_type(val); +/* Count value and string pool sizes; return false on malformed structure or overflow. */ +static bool yyjson_mut_stat(const yyjson_mut_val *val, + usize *val_sum, usize *str_sum, + usize depth) { + yyjson_type type; + + if (unlikely(depth >= YYJSON_MAX_DEPTH)) return false; + if (unlikely(!val)) return false; + type = unsafe_yyjson_get_type(val); + if (unlikely(size_add_is_overflow(*val_sum, 1))) return false; *val_sum += 1; + if (type == YYJSON_TYPE_ARR || type == YYJSON_TYPE_OBJ) { yyjson_mut_val *child = (yyjson_mut_val *)val->uni.ptr; usize len = unsafe_yyjson_get_len(val), i; - len <<= (u8)(type == YYJSON_TYPE_OBJ); + + if (type == YYJSON_TYPE_OBJ) { + if (unlikely(len > USIZE_MAX / 2)) return false; + len <<= 1; + } + + if (unlikely(len > 0 && !child)) return false; + if (unlikely(size_add_is_overflow(*val_sum, len))) return false; *val_sum += len; + for (i = 0; i < len; i++) { yyjson_type stype = unsafe_yyjson_get_type(child); if (stype == YYJSON_TYPE_STR || stype == YYJSON_TYPE_RAW) { - *str_sum += unsafe_yyjson_get_len(child) + 1; + usize str_len = unsafe_yyjson_get_len(child); + if (unlikely(size_add_is_overflow(str_len, 1) || + size_add_is_overflow(*str_sum, str_len + 1))) { + return false; + } + *str_sum += str_len + 1; } else if (stype == YYJSON_TYPE_ARR || stype == YYJSON_TYPE_OBJ) { - yyjson_mut_stat(child, val_sum, str_sum); + if (unlikely(!yyjson_mut_stat(child, val_sum, str_sum, + depth + 1))) { + return false; + } *val_sum -= 1; } - child = child->next; + + if (likely(i + 1 < len)) { + child = child->next; + if (unlikely(!child)) return false; + } } } else if (type == YYJSON_TYPE_STR || type == YYJSON_TYPE_RAW) { - *str_sum += unsafe_yyjson_get_len(val) + 1; + usize len = unsafe_yyjson_get_len(val); + if (unlikely(size_add_is_overflow(len, 1) || + size_add_is_overflow(*str_sum, len + 1))) { + return false; + } + *str_sum += len + 1; } + + return true; } /* Copy mutable values to immutable value pool. */ static usize yyjson_imut_copy(yyjson_val **val_ptr, char **buf_ptr, - const yyjson_mut_val *mval) { + const yyjson_mut_val *mval, + usize depth) { yyjson_val *val = *val_ptr; - yyjson_type type = unsafe_yyjson_get_type(mval); + yyjson_type type; + + if (unlikely(depth >= YYJSON_MAX_DEPTH)) return 0; + if (unlikely(!mval)) return 0; + type = unsafe_yyjson_get_type(mval); + if (type == YYJSON_TYPE_ARR || type == YYJSON_TYPE_OBJ) { yyjson_mut_val *child = (yyjson_mut_val *)mval->uni.ptr; usize len = unsafe_yyjson_get_len(mval), i; usize val_sum = 1; + if (type == YYJSON_TYPE_OBJ) { - if (len) child = child->next->next; + if (unlikely(len > USIZE_MAX / 2)) return 0; + if (len) { + if (unlikely(!child || !child->next)) return 0; + child = child->next->next; + } len <<= 1; } else { - if (len) child = child->next; + if (len) { + if (unlikely(!child || !child->next)) return 0; + child = child->next; + } } + *val_ptr = val + 1; for (i = 0; i < len; i++) { - val_sum += yyjson_imut_copy(val_ptr, buf_ptr, child); - child = child->next; + usize child_sum = yyjson_imut_copy(val_ptr, buf_ptr, child, + depth + 1); + if (unlikely(child_sum == 0 || + size_add_is_overflow(val_sum, child_sum))) { + return 0; + } + val_sum += child_sum; + + if (likely(i + 1 < len)) { + child = child->next; + if (unlikely(!child)) return 0; + } } val->tag = mval->tag; + if (unlikely(size_mul_is_overflow(val_sum, sizeof(yyjson_val)))) { + return 0; + } val->uni.ofs = val_sum * sizeof(yyjson_val); return val_sum; } else if (type == YYJSON_TYPE_STR || type == YYJSON_TYPE_RAW) { char *buf = *buf_ptr; usize len = unsafe_yyjson_get_len(mval); - memcpy((void *)buf, (const void *)mval->uni.str, len); + + if (unlikely(!buf || (len > 0 && !mval->uni.str))) return 0; + if (len > 0) memcpy((void *)buf, (const void *)mval->uni.str, len); buf[len] = '\0'; val->tag = mval->tag; val->uni.str = buf; *val_ptr = val + 1; + if (unlikely(size_add_is_overflow(len, 1))) return 0; *buf_ptr = buf + len + 1; return 1; } else { @@ -2876,9 +2952,14 @@ yyjson_doc *yyjson_mut_doc_imut_copy(const yyjson_mut_doc *mdoc, return yyjson_mut_val_imut_copy(mdoc->root, alc); } +/* + Convert a mutable value tree to immutable document. + Returns NULL if the input structure is malformed, memory size arithmetic + overflows are detected, or the recursion depth limit is exceeded. + */ yyjson_doc *yyjson_mut_val_imut_copy(const yyjson_mut_val *mval, const yyjson_alc *alc) { - usize val_num = 0, str_sum = 0, hdr_size, buf_size; + usize val_num = 0, str_sum = 0, hdr_size, val_size, buf_size; yyjson_doc *doc = NULL; yyjson_val *val_hdr = NULL; @@ -2889,11 +2970,15 @@ yyjson_doc *yyjson_mut_val_imut_copy(const yyjson_mut_val *mval, if (!alc) alc = &YYJSON_DEFAULT_ALC; /* traverse the input value to get pool size */ - yyjson_mut_stat(mval, &val_num, &str_sum); + if (unlikely(!yyjson_mut_stat(mval, &val_num, &str_sum, 0))) return NULL; + if (unlikely(size_add_is_overflow(str_sum, 1))) return NULL; /* create doc and val pool */ hdr_size = size_align_up(sizeof(yyjson_doc), sizeof(yyjson_val)); - buf_size = hdr_size + val_num * sizeof(yyjson_val); + if (unlikely(size_mul_is_overflow(val_num, sizeof(yyjson_val)))) return NULL; + val_size = val_num * sizeof(yyjson_val); + if (unlikely(size_add_is_overflow(hdr_size, val_size))) return NULL; + buf_size = hdr_size + val_size; doc = (yyjson_doc *)alc->malloc(alc->ctx, buf_size); if (!doc) return NULL; memset(doc, 0, sizeof(yyjson_doc)); @@ -2912,7 +2997,12 @@ yyjson_doc *yyjson_mut_val_imut_copy(const yyjson_mut_val *mval, } /* copy vals and strs */ - doc->val_read = yyjson_imut_copy(&val_hdr, &str_hdr, mval); + doc->val_read = yyjson_imut_copy(&val_hdr, &str_hdr, mval, 0); + if (unlikely(doc->val_read == 0)) { + if (doc->str_pool) alc->free(alc->ctx, doc->str_pool); + alc->free(alc->ctx, (void *)doc); + return NULL; + } doc->dat_read = str_sum + 1; return doc; } diff --git a/test/test_json_mut_val.c b/test/test_json_mut_val.c index d810607..f6c10d7 100644 --- a/test/test_json_mut_val.c +++ b/test/test_json_mut_val.c @@ -2150,7 +2150,57 @@ static void test_json_mut_doc_api_one(const char *json_str) { } #endif +static void test_json_mut_doc_imut_copy_invalid(void) { + yyjson_mut_val arr, obj, key, str; + + memset(&arr, 0, sizeof(arr)); + memset(&obj, 0, sizeof(obj)); + memset(&key, 0, sizeof(key)); + memset(&str, 0, sizeof(str)); + + unsafe_yyjson_set_arr(&arr, 1); + arr.uni.ptr = NULL; + yy_assert(!yyjson_mut_val_imut_copy(&arr, NULL)); + + unsafe_yyjson_set_obj(&obj, 1); + unsafe_yyjson_set_str(&key, "k"); + key.next = NULL; + obj.uni.ptr = &key; + yy_assert(!yyjson_mut_val_imut_copy(&obj, NULL)); + + unsafe_yyjson_set_strn(&str, NULL, 1); + yy_assert(!yyjson_mut_val_imut_copy(&str, NULL)); +} + +static void test_json_mut_doc_imut_copy_deep_nesting(void) { + yyjson_mut_doc *doc = yyjson_mut_doc_new(NULL); + yyjson_mut_val *root; + yyjson_mut_val *cur; + yyjson_doc *idoc; + + yy_assert(doc); + root = yyjson_mut_arr(doc); + yy_assert(root); + cur = root; + + /* Depth 2000 exceeds YYJSON_MAX_DEPTH and should fail safely. */ + for (int i = 0; i < 2000; i++) { + yyjson_mut_val *child = yyjson_mut_arr(doc); + yy_assert(child); + yy_assert(yyjson_mut_arr_append(cur, child)); + cur = child; + } + + idoc = yyjson_mut_val_imut_copy(root, NULL); + yy_assert(!idoc); + + yyjson_mut_doc_free(doc); +} + static void test_json_mut_doc_api(void) { + test_json_mut_doc_imut_copy_invalid(); + test_json_mut_doc_imut_copy_deep_nesting(); + { yyjson_mut_doc_set_root(NULL, NULL); yy_assert(yyjson_mut_doc_get_root(NULL) == NULL); From c396bdba9e9afe9e3eab82d934c82dc623932e32 Mon Sep 17 00:00:00 2001 From: rootvector2 Date: Mon, 13 Apr 2026 22:05:29 +0530 Subject: [PATCH 2/2] Align imut copy hardening with maintainer guidance --- src/yyjson.c | 59 ++++++++-------------------------------- test/test_json_mut_val.c | 50 ---------------------------------- 2 files changed, 12 insertions(+), 97 deletions(-) diff --git a/src/yyjson.c b/src/yyjson.c index 5cfd0bc..51025ab 100644 --- a/src/yyjson.c +++ b/src/yyjson.c @@ -521,12 +521,6 @@ uint32_t yyjson_version(void) { /* maximum length of a number in incremental parsing */ #define INCR_NUM_MAX_LEN 1024 -/* - Maximum recursion depth for mutable value traversal and copy. - This prevents stack overflow when processing deeply nested or malicious inputs. - */ -#define YYJSON_MAX_DEPTH 1024 - /*============================================================================== @@ -2815,14 +2809,11 @@ yyjson_mut_val *yyjson_mut_val_mut_copy(yyjson_mut_doc *doc, return NULL; } -/* Count value and string pool sizes; return false on malformed structure or overflow. */ +/* Count value and string pool sizes; return false on overflow. */ static bool yyjson_mut_stat(const yyjson_mut_val *val, - usize *val_sum, usize *str_sum, - usize depth) { + usize *val_sum, usize *str_sum) { yyjson_type type; - if (unlikely(depth >= YYJSON_MAX_DEPTH)) return false; - if (unlikely(!val)) return false; type = unsafe_yyjson_get_type(val); if (unlikely(size_add_is_overflow(*val_sum, 1))) return false; *val_sum += 1; @@ -2836,7 +2827,6 @@ static bool yyjson_mut_stat(const yyjson_mut_val *val, len <<= 1; } - if (unlikely(len > 0 && !child)) return false; if (unlikely(size_add_is_overflow(*val_sum, len))) return false; *val_sum += len; @@ -2850,17 +2840,12 @@ static bool yyjson_mut_stat(const yyjson_mut_val *val, } *str_sum += str_len + 1; } else if (stype == YYJSON_TYPE_ARR || stype == YYJSON_TYPE_OBJ) { - if (unlikely(!yyjson_mut_stat(child, val_sum, str_sum, - depth + 1))) { + if (unlikely(!yyjson_mut_stat(child, val_sum, str_sum))) { return false; } *val_sum -= 1; } - - if (likely(i + 1 < len)) { - child = child->next; - if (unlikely(!child)) return false; - } + child = child->next; } } else if (type == YYJSON_TYPE_STR || type == YYJSON_TYPE_RAW) { usize len = unsafe_yyjson_get_len(val); @@ -2876,13 +2861,10 @@ static bool yyjson_mut_stat(const yyjson_mut_val *val, /* Copy mutable values to immutable value pool. */ static usize yyjson_imut_copy(yyjson_val **val_ptr, char **buf_ptr, - const yyjson_mut_val *mval, - usize depth) { + const yyjson_mut_val *mval) { yyjson_val *val = *val_ptr; yyjson_type type; - if (unlikely(depth >= YYJSON_MAX_DEPTH)) return 0; - if (unlikely(!mval)) return 0; type = unsafe_yyjson_get_type(mval); if (type == YYJSON_TYPE_ARR || type == YYJSON_TYPE_OBJ) { @@ -2892,32 +2874,21 @@ static usize yyjson_imut_copy(yyjson_val **val_ptr, char **buf_ptr, if (type == YYJSON_TYPE_OBJ) { if (unlikely(len > USIZE_MAX / 2)) return 0; - if (len) { - if (unlikely(!child || !child->next)) return 0; - child = child->next->next; - } + if (len) child = child->next->next; len <<= 1; } else { - if (len) { - if (unlikely(!child || !child->next)) return 0; - child = child->next; - } + if (len) child = child->next; } *val_ptr = val + 1; for (i = 0; i < len; i++) { - usize child_sum = yyjson_imut_copy(val_ptr, buf_ptr, child, - depth + 1); + usize child_sum = yyjson_imut_copy(val_ptr, buf_ptr, child); if (unlikely(child_sum == 0 || size_add_is_overflow(val_sum, child_sum))) { return 0; } val_sum += child_sum; - - if (likely(i + 1 < len)) { - child = child->next; - if (unlikely(!child)) return 0; - } + child = child->next; } val->tag = mval->tag; if (unlikely(size_mul_is_overflow(val_sum, sizeof(yyjson_val)))) { @@ -2929,8 +2900,7 @@ static usize yyjson_imut_copy(yyjson_val **val_ptr, char **buf_ptr, char *buf = *buf_ptr; usize len = unsafe_yyjson_get_len(mval); - if (unlikely(!buf || (len > 0 && !mval->uni.str))) return 0; - if (len > 0) memcpy((void *)buf, (const void *)mval->uni.str, len); + memcpy((void *)buf, (const void *)mval->uni.str, len); buf[len] = '\0'; val->tag = mval->tag; val->uni.str = buf; @@ -2952,11 +2922,6 @@ yyjson_doc *yyjson_mut_doc_imut_copy(const yyjson_mut_doc *mdoc, return yyjson_mut_val_imut_copy(mdoc->root, alc); } -/* - Convert a mutable value tree to immutable document. - Returns NULL if the input structure is malformed, memory size arithmetic - overflows are detected, or the recursion depth limit is exceeded. - */ yyjson_doc *yyjson_mut_val_imut_copy(const yyjson_mut_val *mval, const yyjson_alc *alc) { usize val_num = 0, str_sum = 0, hdr_size, val_size, buf_size; @@ -2970,7 +2935,7 @@ yyjson_doc *yyjson_mut_val_imut_copy(const yyjson_mut_val *mval, if (!alc) alc = &YYJSON_DEFAULT_ALC; /* traverse the input value to get pool size */ - if (unlikely(!yyjson_mut_stat(mval, &val_num, &str_sum, 0))) return NULL; + if (unlikely(!yyjson_mut_stat(mval, &val_num, &str_sum))) return NULL; if (unlikely(size_add_is_overflow(str_sum, 1))) return NULL; /* create doc and val pool */ @@ -2997,7 +2962,7 @@ yyjson_doc *yyjson_mut_val_imut_copy(const yyjson_mut_val *mval, } /* copy vals and strs */ - doc->val_read = yyjson_imut_copy(&val_hdr, &str_hdr, mval, 0); + doc->val_read = yyjson_imut_copy(&val_hdr, &str_hdr, mval); if (unlikely(doc->val_read == 0)) { if (doc->str_pool) alc->free(alc->ctx, doc->str_pool); alc->free(alc->ctx, (void *)doc); diff --git a/test/test_json_mut_val.c b/test/test_json_mut_val.c index f6c10d7..d810607 100644 --- a/test/test_json_mut_val.c +++ b/test/test_json_mut_val.c @@ -2150,57 +2150,7 @@ static void test_json_mut_doc_api_one(const char *json_str) { } #endif -static void test_json_mut_doc_imut_copy_invalid(void) { - yyjson_mut_val arr, obj, key, str; - - memset(&arr, 0, sizeof(arr)); - memset(&obj, 0, sizeof(obj)); - memset(&key, 0, sizeof(key)); - memset(&str, 0, sizeof(str)); - - unsafe_yyjson_set_arr(&arr, 1); - arr.uni.ptr = NULL; - yy_assert(!yyjson_mut_val_imut_copy(&arr, NULL)); - - unsafe_yyjson_set_obj(&obj, 1); - unsafe_yyjson_set_str(&key, "k"); - key.next = NULL; - obj.uni.ptr = &key; - yy_assert(!yyjson_mut_val_imut_copy(&obj, NULL)); - - unsafe_yyjson_set_strn(&str, NULL, 1); - yy_assert(!yyjson_mut_val_imut_copy(&str, NULL)); -} - -static void test_json_mut_doc_imut_copy_deep_nesting(void) { - yyjson_mut_doc *doc = yyjson_mut_doc_new(NULL); - yyjson_mut_val *root; - yyjson_mut_val *cur; - yyjson_doc *idoc; - - yy_assert(doc); - root = yyjson_mut_arr(doc); - yy_assert(root); - cur = root; - - /* Depth 2000 exceeds YYJSON_MAX_DEPTH and should fail safely. */ - for (int i = 0; i < 2000; i++) { - yyjson_mut_val *child = yyjson_mut_arr(doc); - yy_assert(child); - yy_assert(yyjson_mut_arr_append(cur, child)); - cur = child; - } - - idoc = yyjson_mut_val_imut_copy(root, NULL); - yy_assert(!idoc); - - yyjson_mut_doc_free(doc); -} - static void test_json_mut_doc_api(void) { - test_json_mut_doc_imut_copy_invalid(); - test_json_mut_doc_imut_copy_deep_nesting(); - { yyjson_mut_doc_set_root(NULL, NULL); yy_assert(yyjson_mut_doc_get_root(NULL) == NULL);