From 3ccbfcf4efd2a7d020c068490a7348636405288a Mon Sep 17 00:00:00 2001
From: Florian Knigge <flknigge@gmail.com>
Date: Tue, 22 Oct 2024 12:56:02 +0200
Subject: [PATCH] Make errors in H replacement more resilient

Free the accum_field and subfields always if an error occurs during field replacement (in a list) and they were malloced
---
 ccronexpr.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/ccronexpr.c b/ccronexpr.c
index 449b126..149bff0 100644
--- a/ccronexpr.c
+++ b/ccronexpr.c
@@ -1339,15 +1339,17 @@ static char* check_and_replace_h(char* field, unsigned int pos, unsigned int min
 {
     char* has_h = strchr(field, 'H');
     if (has_h) {
+        char  *accum_field = NULL;
+        char **subfields = NULL;
+        size_t subfields_len = 0;
         // Check if Field contains ',', if so, split into multiple subfields, and replace in each (with same position no)
         char *has_comma = strchr(field, ',');
         if (has_comma) {
             // Iterate over split sub-fields, check for 'H' and replace if present
-            size_t subfields_len = 0;
-            char** subfields = split_str(field, ',', &subfields_len);
+            subfields = split_str(field, ',', &subfields_len);
             if (subfields == NULL) {
                 *error = "Failed to split 'H' string in list";
-                return field;
+                goto return_error;
             }
             size_t res_len = 0;
             size_t res_lens[subfields_len];
@@ -1357,21 +1359,25 @@ static char* check_and_replace_h(char* field, unsigned int pos, unsigned int min
                     subfields[i] = replace_h_entry(subfields[i], pos, min, error);
                 }
                 if (*error != NULL) {
-                    free_splitted(subfields, subfields_len);
-                    return field; // something went wrong trying to replace a subfield
+                    goto return_error;
                 }
                 res_lens[i] = strnlen(subfields[i], CRON_MAX_STR_LEN_TO_SPLIT);
                 res_len += res_lens[i];
             }
             // Allocate space for the full string: Result lengths + (result count - 1) for the commas + 1 for '\0'
-            char *accum_field = (char *) cronMalloc(res_len + subfields_len );
+            accum_field = (char *) cronMalloc(res_len + subfields_len );
             if (accum_field == NULL) {
                 *error = "Failed to merge 'H' in list";
-                return field;
+                goto return_error;
             }
             memset(accum_field, 0, res_len + subfields_len);
             char *tracking = accum_field;
             for (size_t i = 0; i < subfields_len; i++) {
+                // Sanity check: Is "tracking" still in the allocated memory boundaries?
+                if ((tracking - accum_field) > (res_len + subfields_len)) {
+                    *error = "Failed to insert subfields to merged fields: String went oob";
+                    goto return_error;
+                }
                 strncpy(tracking, subfields[i], res_lens[i]);
                 tracking += res_lens[i];
                 // Don't append comma to last list entry
@@ -1386,6 +1392,12 @@ static char* check_and_replace_h(char* field, unsigned int pos, unsigned int min
         }
         // only one H to find and replace, then return
         field = replace_h_entry(field, pos, min, error);
+        return field;
+
+        return_error:
+        if (subfields) free_splitted(subfields, subfields_len);
+        if (accum_field) cronFree(accum_field);
+        return field;
     }
     return field;
 }