From 5b7a8606b062baa48b98352007cccacd8050fbfc Mon Sep 17 00:00:00 2001 From: Aaran McGuire Date: Tue, 5 May 2026 19:54:56 +0100 Subject: [PATCH] =?UTF-8?q?Fix=20off-by-one=20in=20label/key=20malloc=20?= =?UTF-8?q?=E2=80=94=20missing=20NUL=20terminator?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit readstat_copy_label() allocates strlen(label) bytes and memcpy's strlen(label) bytes — no NUL terminator. Any consumer calling strlen() on the stored label reads past the end of the allocation. readstat_label_string_value() has the same bug for string_key: it stores the key string without a NUL terminator. Both allocations now use strlen + 1 and copy strlen + 1 bytes. readstat_copy_label() changes return type to readstat_error_t to propagate allocation failure. --- src/readstat_writer.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/readstat_writer.c b/src/readstat_writer.c index 82219f2b..5f3ba44f 100644 --- a/src/readstat_writer.c +++ b/src/readstat_writer.c @@ -75,12 +75,14 @@ static void readstat_label_set_free(readstat_label_set_t *label_set) { free(label_set); } -static void readstat_copy_label(readstat_value_label_t *value_label, const char *label) { +static readstat_error_t readstat_copy_label(readstat_value_label_t *value_label, const char *label) { if (label && strlen(label)) { value_label->label_len = strlen(label); - value_label->label = malloc(value_label->label_len); - memcpy(value_label->label, label, value_label->label_len); + value_label->label = malloc(value_label->label_len + 1); + if (value_label->label == NULL) return READSTAT_ERROR_MALLOC; + memcpy(value_label->label, label, value_label->label_len + 1); } + return READSTAT_OK; } static readstat_value_label_t *readstat_add_value_label(readstat_label_set_t *label_set, const char *label) { @@ -91,7 +93,8 @@ static readstat_value_label_t *readstat_add_value_label(readstat_label_set_t *la } readstat_value_label_t *new_value_label = &label_set->value_labels[label_set->value_labels_count++]; memset(new_value_label, 0, sizeof(readstat_value_label_t)); - readstat_copy_label(new_value_label, label); + if (readstat_copy_label(new_value_label, label) != READSTAT_OK) + return NULL; return new_value_label; } @@ -355,8 +358,9 @@ void readstat_label_string_value(readstat_label_set_t *label_set, const char *va readstat_value_label_t *new_value_label = readstat_add_value_label(label_set, label); if (value && strlen(value)) { new_value_label->string_key_len = strlen(value); - new_value_label->string_key = malloc(new_value_label->string_key_len); - memcpy(new_value_label->string_key, value, new_value_label->string_key_len); + new_value_label->string_key = malloc(new_value_label->string_key_len + 1); + if (new_value_label->string_key == NULL) return; + memcpy(new_value_label->string_key, value, new_value_label->string_key_len + 1); } }