From 46cc183fdcacd549ca3e545215ce610b5590fa8f Mon Sep 17 00:00:00 2001 From: bmryggdrasil <120229435+bmryggdrasil@users.noreply.github.com> Date: Fri, 6 Nov 2026 05:34:23 +0800 Subject: [PATCH] Check realloc results before using the grown buffer Four places still assign realloc's result straight back to the only pointer to the old block, so a failed realloc leaks the old block and the code then writes through NULL: - ck_str_n_hash_insert doubles keys_capacity and then copies the new key to table->keys + keys_used. Since keys_used is the size of all earlier keys, the write lands at an offset from NULL rather than at NULL itself. count was also bumped before the copy. - txt_getdelim doubles value_buffer_len inside the realloc call and keeps reading into value_buffer[i], again at an offset from NULL. - sas7bcat_parse_value_labels passes the NULL label buffer to readstat_convert. - sav_emit_long_string_value_labels_record memsets the NULL space_buffer. Keep the old pointer until realloc succeeds and return READSTAT_ERROR_MALLOC (or 0 from the hash insert, which callers already treat as failure) so the existing cleanup paths free it. txt_getdelim returns -2 for allocation failure so txt_parse_delimited can tell it apart from a read error. The key-buffer growth in the hash table also refuses to double past SIZE_MAX. This continues #373, which fixed the same pattern in other readers. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/CKHashTable.c | 17 +++++++++++++---- src/sas/readstat_sas7bcat_read.c | 7 ++++++- src/spss/readstat_sav_write.c | 7 ++++++- src/txt/readstat_txt_read.c | 13 ++++++++++++- 4 files changed, 37 insertions(+), 7 deletions(-) diff --git a/src/CKHashTable.c b/src/CKHashTable.c index bf826dc0..0014ec63 100644 --- a/src/CKHashTable.c +++ b/src/CKHashTable.c @@ -227,11 +227,20 @@ int ck_str_n_hash_insert(const char *key, size_t keylen, const void *value, ck_h ck_hash_entry_t *entry = &table->entries[hash_key]; char *this_key = &table->keys[entry->key_offset]; if (entry->key_length == 0) { - table->count++; - while (table->keys_used + keylen > table->keys_capacity) { - table->keys_capacity *= 2; - table->keys = realloc(table->keys, table->keys_capacity); + if (table->keys_used + keylen > table->keys_capacity) { + size_t new_capacity = table->keys_capacity; + while (table->keys_used + keylen > new_capacity) { + if (new_capacity > SIZE_MAX / 2) + return 0; + new_capacity *= 2; + } + char *new_keys = realloc(table->keys, new_capacity); + if (new_keys == NULL) + return 0; + table->keys = new_keys; + table->keys_capacity = new_capacity; } + table->count++; memcpy(table->keys + table->keys_used, key, keylen); entry->key_offset = table->keys_used; entry->key_length = keylen; diff --git a/src/sas/readstat_sas7bcat_read.c b/src/sas/readstat_sas7bcat_read.c index 1d9c08eb..bd863aeb 100644 --- a/src/sas/readstat_sas7bcat_read.c +++ b/src/sas/readstat_sas7bcat_read.c @@ -163,7 +163,12 @@ static readstat_error_t sas7bcat_parse_value_labels(const char *value_start, siz label_len = value_start + value_labels_len - &lbp2[10]; } if (ctx->value_label_handler && !is_default_label) { - label = realloc(label, 4 * label_len + 1); + char *new_label = realloc(label, 4 * label_len + 1); + if (new_label == NULL) { + retval = READSTAT_ERROR_MALLOC; + goto cleanup; + } + label = new_label; retval = readstat_convert(label, 4 * label_len + 1, &lbp2[10], label_len, ctx->converter); if (retval != READSTAT_OK) diff --git a/src/spss/readstat_sav_write.c b/src/spss/readstat_sav_write.c index 61c5c7a1..795d67d7 100644 --- a/src/spss/readstat_sav_write.c +++ b/src/spss/readstat_sav_write.c @@ -993,7 +993,12 @@ static readstat_error_t sav_emit_long_string_value_labels_record(readstat_writer if (storage_width <= 8) continue; - space_buffer = realloc(space_buffer, user_width); + char *new_space_buffer = realloc(space_buffer, user_width); + if (new_space_buffer == NULL) { + retval = READSTAT_ERROR_MALLOC; + goto cleanup; + } + space_buffer = new_space_buffer; memset(space_buffer, ' ', user_width); retval = readstat_write_bytes(writer, &name_len, sizeof(int32_t)); diff --git a/src/txt/readstat_txt_read.c b/src/txt/readstat_txt_read.c index e48f687a..4ecd81ae 100644 --- a/src/txt/readstat_txt_read.c +++ b/src/txt/readstat_txt_read.c @@ -58,7 +58,14 @@ static ssize_t txt_getdelim(char ** restrict linep, size_t * restrict linecapp, ssize_t bytes_read = 0; while ((bytes_read = io->read(&value_buffer[i], 1, io->io_ctx)) == 1 && value_buffer[i++] != delimiter) { if (i == value_buffer_len) { - value_buffer = realloc(value_buffer, value_buffer_len *= 2); + char *new_value_buffer = realloc(value_buffer, value_buffer_len * 2); + if (new_value_buffer == NULL) { + *linep = value_buffer; + *linecapp = value_buffer_len; + return -2; + } + value_buffer = new_value_buffer; + value_buffer_len *= 2; } } *linep = value_buffer; @@ -91,6 +98,10 @@ static readstat_error_t txt_parse_delimited(readstat_parser_t *parser, retval = READSTAT_ERROR_READ; goto cleanup; } + if (chars_read == -2) { + retval = READSTAT_ERROR_MALLOC; + goto cleanup; + } if (parser->handlers.value && !entry->skip) { chars_read--; // delimiter if (chars_read > 0 && value_buffer[chars_read-1] == '\r') {