[bg0d-droid]

Summary

This PR fixes a double-free reachable from XML parsing and systematically hardens allocation-failure handling across the library, replacing assert()-based error handling and unchecked size arithmetic with proper error propagation.

1. Double-free in node_from_xml (src/xplist.c)

node_from_xml transfers ownership of a freshly created node to the tree (plist_dict_set_item / plist_array_append_item / *plist = subnode), but only cleared subnode at the very end of the block. When an error occurred in between — concretely, the node_path_item allocation failing right after the root object was assigned to *plist — err_out freed the node while the tree still referenced it, and plist_free(*plist) then freed it again:

free(): double free detected in tcache 2

Reproduced with a malloc fault-injection harness (see Verification below): parsing a small XML plist with the failure landing on that allocation aborts on unpatched master with the message above; the core trace shows plist_free → plist_free_node → node_destroy → free() aborting during the err_out cleanup at xplist.c:1618.

The fix transfers ownership explicitly (subnode = NULL) at each attach point, so err_out can never free a node the tree owns. The two error paths after attach also now report PLIST_ERR_NO_MEM instead of PLIST_ERR_PARSE when the allocation fails.

2. Allocation failures crash before callers can handle them

Every caller of byte_array_new, ptr_array_new and hash_table_new already checks the result for NULL and reports PLIST_ERR_NO_MEM — e.g. plist_from_bin:

bplist.used_indexes = ptr_array_new(16);
if (!bplist.used_indexes) {
    return PLIST_ERR_NO_MEM;
}

But the constructors dereference the malloc result before returning, so those checks are unreachable dead code and the library crashes instead. The constructors now honor the contract every caller already assumes, in bytearray.c, ptrarray.c, hashtable.c (both hash_table_new and hash_table_insert).

The same inconsistency existed in the XML engine: text_part_append and text_parts_get_content guarded allocations with assert() (compiled out entirely with NDEBUG, so release builds write through NULL) or nothing at all. Both now return NULL cleanly and get_text_parts propagates PLIST_ERR_NO_MEM. The remaining unchecked malloc(64) / malloc(slen+1) allocations in the OpenStep and JSON engines are checked as well.

3. Integer-overflow guards on size arithmetic

• byte_array_new / byte_array_grow: page-size rounding and capacity + increase are now checked before use; overflow puts the array into the same well-defined failed state as OOM.
• byte_array_append: guards len against SIZE_MAX - ba->len.
• ptr_array_new / ptr_array_insert: allocation and realloc sizes checked for int and size_t overflow.
• bplist.c: the UTF-16→UTF-8 (4*(len+1)) and UTF-8→UTF-16 (((size*2)+1)*sizeof(uint16_t)) conversion buffer computations are checked against overflow.

4. Wrong bounds check in the XML base64 write path

The buffer guard for base64encode output was:

assert((*outbuf)->len + count < (*outbuf)->capacity);

count is the raw input byte count; base64encode writes 4*ceil(count/3) bytes plus a terminating NUL. The assert checked the wrong quantity and, compiled out under NDEBUG, left the only bounds check on that write path missing entirely. It is now a real runtime check against the actual encoded size, growing the buffer or returning PLIST_ERR_NO_MEM.

Verification

Fault-injection sweep — an LD_PRELOAD interposer failing the k-th allocation (armed from main()), swept over 250 failure points × 2 conversion directions × XML/binary/JSON/OpenStep inputs (419 runs per build):

build	signal deaths
unpatched	40 (SIGSEGV + SIGABRT, incl. the double-free above)
patched	0

All failure points now produce clean PLIST_ERR_NO_MEM errors.

Test suite — all 43 tests pass on a normal build and under -fsanitize=address,undefined. (The suite initially had one pre-existing leak in test/integer_set.c under LeakSanitizer, fixed in the second commit.)

Formatting — the project runs clang-format via pre-commit; touched hunks follow each file's existing style.

Security impact

The double-free is memory corruption in a parser that routinely processes untrusted input (mobile device backups, plists from the network). Beyond that specific fix, the patch eliminates the entire error-prone pattern: assert() as the only guard on allocation results (disappears in release builds), unchecked allocation returns that their callers already expected to check, and unchecked size arithmetic in growth/conversion paths — so allocation failure now degrades to a clean error instead of undefined behavior.