log2journal: fix config parsing memory leaks (#18893)

* fix memory leaks

* cleanup parsing of booleans

src/collectors/log2journal/log2journal-params.c

@@ -47,6 +47,12 @@ void log_job_cleanup(LOG_JOB *jb) {
     for(size_t i = 0; i < jb->rewrites.used; i++)
         rewrite_cleanup(&jb->rewrites.array[i]);
 
+    search_pattern_cleanup(&jb->filter.include);
+    search_pattern_cleanup(&jb->filter.exclude);
+
+    hashed_key_cleanup(&jb->filename.key);
+    hashed_key_cleanup(&jb->unmatched.key);
+
     txt_cleanup(&jb->rewrites.tmp);
     txt_cleanup(&jb->filename.current);
 

src/collectors/log2journal/log2journal-pcre2.c

@@ -102,8 +102,15 @@ PCRE2_STATE *pcre2_parser_create(LOG_JOB *jb) {
 }
 
 void pcre2_parser_destroy(PCRE2_STATE *pcre2) {
-    if(pcre2)
+    if(pcre2) {
+        if(pcre2->re)
+            pcre2_code_free(pcre2->re);
+
+        if(pcre2->match_data)
+            pcre2_match_data_free(pcre2->match_data);
+
         freez(pcre2);
+    }
 }
 
 const char *pcre2_parser_error(PCRE2_STATE *pcre2) {

src/collectors/log2journal/log2journal-rewrite.c

@@ -7,6 +7,7 @@ void rewrite_cleanup(REWRITE *rw) {
 
     if(rw->flags & RW_MATCH_PCRE2)
         search_pattern_cleanup(&rw->match_pcre2);
+
     else if(rw->flags & RW_MATCH_NON_EMPTY)
         replace_pattern_cleanup(&rw->match_non_empty);
 

src/collectors/log2journal/log2journal-yaml.c

@@ -280,6 +280,8 @@ static bool yaml_parse_constant_field_injection(yaml_parser_t *parser, LOG_JOB *
         goto cleanup;
     }
 
+    yaml_event_delete(&event);
+
     if (!yaml_parse(parser, &event) || event.type != YAML_SCALAR_EVENT) {
         yaml_error(parser, &event, "Expected scalar for constant field injection value");
         goto cleanup;
@@ -315,7 +317,7 @@ static bool yaml_parse_injection_mapping(yaml_parser_t *parser, LOG_JOB *jb, boo
         switch (event.type) {
             case YAML_SCALAR_EVENT:
                 if (yaml_scalar_matches(&event, "key", strlen("key"))) {
-                    errors += yaml_parse_constant_field_injection(parser, jb, unmatched);
+                    errors += yaml_parse_constant_field_injection(parser, jb, unmatched) ? 1 : 0;
                 } else {
                     yaml_error(parser, &event, "Unexpected scalar in injection mapping");
                     errors++;
@@ -427,6 +429,149 @@ static size_t yaml_parse_unmatched(yaml_parser_t *parser, LOG_JOB *jb) {
     return errors;
 }
 
+static bool yaml_parse_scalar_boolean(yaml_parser_t *parser, bool def, const char *where, size_t *errors) {
+    bool rc = def;
+
+    yaml_event_t value_event;
+    if (!yaml_parse(parser, &value_event)) {
+        (*errors)++;
+        return rc;
+    }
+
+    if (value_event.type != YAML_SCALAR_EVENT) {
+        yaml_error(parser, &value_event, "Expected scalar for %s boolean", where);
+        (*errors)++;
+    }
+    else if(strncmp((char*)value_event.data.scalar.value, "yes", 3) == 0 ||
+             strncmp((char*)value_event.data.scalar.value, "true", 4) == 0)
+        rc = true;
+    else if(strncmp((char*)value_event.data.scalar.value, "no", 2) == 0 ||
+             strncmp((char*)value_event.data.scalar.value, "false", 5) == 0)
+        rc = false;
+    else {
+        yaml_error(parser, &value_event, "Expected scalar for %s boolean: invalid value %s", where, value_event.data.scalar.value);
+        rc = def;
+    }
+
+    yaml_event_delete(&value_event);
+    return rc;
+}
+
+static bool handle_rewrite_event(yaml_parser_t *parser, yaml_event_t *event,
+                                 char **key, char **search_pattern, char **replace_pattern,
+                                 RW_FLAGS *flags, bool *mapping_finished,
+                                 LOG_JOB *jb, size_t *errors) {
+    switch (event->type) {
+        case YAML_SCALAR_EVENT:
+            if (yaml_scalar_matches(event, "key", strlen("key"))) {
+                yaml_event_t value_event;
+                if (!yaml_parse(parser, &value_event)) {
+                    (*errors)++;
+                    return false;
+                }
+
+                if (value_event.type != YAML_SCALAR_EVENT) {
+                    yaml_error(parser, &value_event, "Expected scalar for rewrite key");
+                    (*errors)++;
+                } else {
+                    freez(*key);
+                    *key = strndupz((char *)value_event.data.scalar.value, value_event.data.scalar.length);
+                }
+                yaml_event_delete(&value_event);
+            }
+            else if (yaml_scalar_matches(event, "match", strlen("match"))) {
+                yaml_event_t value_event;
+                if (!yaml_parse(parser, &value_event)) {
+                    (*errors)++;
+                    return false;
+                }
+
+                if (value_event.type != YAML_SCALAR_EVENT) {
+                    yaml_error(parser, &value_event, "Expected scalar for rewrite match PCRE2 pattern");
+                    (*errors)++;
+                }
+                else {
+                    freez(*search_pattern);
+                    *flags |= RW_MATCH_PCRE2;
+                    *flags &= ~RW_MATCH_NON_EMPTY;
+                    *search_pattern = strndupz((char *)value_event.data.scalar.value, value_event.data.scalar.length);
+                }
+                yaml_event_delete(&value_event);
+            }
+            else if (yaml_scalar_matches(event, "not_empty", strlen("not_empty"))) {
+                yaml_event_t value_event;
+                if (!yaml_parse(parser, &value_event)) {
+                    (*errors)++;
+                    return false;
+                }
+
+                if (value_event.type != YAML_SCALAR_EVENT) {
+                    yaml_error(parser, &value_event, "Expected scalar for rewrite not empty condition");
+                    (*errors)++;
+                }
+                else {
+                    freez(*search_pattern);
+                    *flags |= RW_MATCH_NON_EMPTY;
+                    *flags &= ~RW_MATCH_PCRE2;
+                    *search_pattern = strndupz((char *)value_event.data.scalar.value, value_event.data.scalar.length);
+                }
+                yaml_event_delete(&value_event);
+            }
+            else if (yaml_scalar_matches(event, "value", strlen("value"))) {
+                yaml_event_t value_event;
+                if (!yaml_parse(parser, &value_event)) {
+                    (*errors)++;
+                    return false;
+                }
+
+                if (value_event.type != YAML_SCALAR_EVENT) {
+                    yaml_error(parser, &value_event, "Expected scalar for rewrite value");
+                    (*errors)++;
+                } else {
+                    freez(*replace_pattern);
+                    *replace_pattern = strndupz((char *)value_event.data.scalar.value, value_event.data.scalar.length);
+                }
+                yaml_event_delete(&value_event);
+            }
+            else if (yaml_scalar_matches(event, "stop", strlen("stop"))) {
+                if(yaml_parse_scalar_boolean(parser, true, "rewrite stop", errors))
+                    *flags &= ~RW_DONT_STOP;
+                else
+                    *flags |= RW_DONT_STOP;
+            }
+            else if (yaml_scalar_matches(event, "inject", strlen("inject"))) {
+                if(yaml_parse_scalar_boolean(parser, false, "rewrite inject", errors))
+                    *flags |= RW_INJECT;
+                else
+                    *flags &= ~RW_INJECT;
+            }
+            else {
+                yaml_error(parser, event, "Unexpected scalar in rewrite mapping");
+                (*errors)++;
+            }
+            break;
+
+        case YAML_MAPPING_END_EVENT:
+            if(*key) {
+                if (!log_job_rewrite_add(jb, *key, *flags, *search_pattern, *replace_pattern))
+                    (*errors)++;
+            }
+
+            freez(*key);
+            freez(*search_pattern);
+            freez(*replace_pattern);
+            *mapping_finished = true;
+            break;
+
+        default:
+            yaml_error(parser, event, "Unexpected event in rewrite mapping");
+            (*errors)++;
+            break;
+    }
+
+    return true;
+}
+
 static size_t yaml_parse_rewrites(yaml_parser_t *parser, LOG_JOB *jb) {
     size_t errors = 0;
 
@@ -457,120 +602,14 @@ static size_t yaml_parse_rewrites(yaml_parser_t *parser, LOG_JOB *jb) {
                         continue;
                     }
 
-                    switch (sub_event.type) {
-                        case YAML_SCALAR_EVENT:
-                            if (yaml_scalar_matches(&sub_event, "key", strlen("key"))) {
-                                if (!yaml_parse(parser, &sub_event) || sub_event.type != YAML_SCALAR_EVENT) {
-                                    yaml_error(parser, &sub_event, "Expected scalar for rewrite key");
-                                    errors++;
-                                } else {
-                                    freez(key);
-                                    key = strndupz((char *)sub_event.data.scalar.value, sub_event.data.scalar.length);
-                                    yaml_event_delete(&sub_event);
-                                }
-                            } else if (yaml_scalar_matches(&sub_event, "match", strlen("match"))) {
-                                if (!yaml_parse(parser, &sub_event) || sub_event.type != YAML_SCALAR_EVENT) {
-                                    yaml_error(parser, &sub_event, "Expected scalar for rewrite match PCRE2 pattern");
-                                    errors++;
-                                }
-                                else {
-                                    if(search_pattern)
-                                        freez(search_pattern);
-                                    flags |= RW_MATCH_PCRE2;
-                                    flags &= ~RW_MATCH_NON_EMPTY;
-                                    search_pattern = strndupz((char *)sub_event.data.scalar.value, sub_event.data.scalar.length);
-                                    yaml_event_delete(&sub_event);
-                                }
-                            } else if (yaml_scalar_matches(&sub_event, "not_empty", strlen("not_empty"))) {
-                                if (!yaml_parse(parser, &sub_event) || sub_event.type != YAML_SCALAR_EVENT) {
-                                    yaml_error(parser, &sub_event, "Expected scalar for rewrite not empty condition");
-                                    errors++;
-                                }
-                                else {
-                                    if(search_pattern)
-                                        freez(search_pattern);
-                                    flags |= RW_MATCH_NON_EMPTY;
-                                    flags &= ~RW_MATCH_PCRE2;
-                                    search_pattern = strndupz((char *)sub_event.data.scalar.value, sub_event.data.scalar.length);
-                                    yaml_event_delete(&sub_event);
-                                }
-                            } else if (yaml_scalar_matches(&sub_event, "value", strlen("value"))) {
-                                if (!yaml_parse(parser, &sub_event) || sub_event.type != YAML_SCALAR_EVENT) {
-                                    yaml_error(parser, &sub_event, "Expected scalar for rewrite value");
-                                    errors++;
-                                } else {
-                                    freez(replace_pattern);
-                                    replace_pattern = strndupz((char *)sub_event.data.scalar.value, sub_event.data.scalar.length);
-                                    yaml_event_delete(&sub_event);
-                                }
-                            } else if (yaml_scalar_matches(&sub_event, "stop", strlen("stop"))) {
-                                if (!yaml_parse(parser, &sub_event) || sub_event.type != YAML_SCALAR_EVENT) {
-                                    yaml_error(parser, &sub_event, "Expected scalar for rewrite stop boolean");
-                                    errors++;
-                                } else {
-                                    if(strncmp((char*)sub_event.data.scalar.value, "no", 2) == 0 ||
-                                        strncmp((char*)sub_event.data.scalar.value, "false", 5) == 0)
-                                        flags |= RW_DONT_STOP;
-                                    else
-                                        flags &= ~RW_DONT_STOP;
-
-                                    yaml_event_delete(&sub_event);
-                                }
-                            } else if (yaml_scalar_matches(&sub_event, "inject", strlen("inject"))) {
-                                if (!yaml_parse(parser, &sub_event) || sub_event.type != YAML_SCALAR_EVENT) {
-                                    yaml_error(parser, &sub_event, "Expected scalar for rewrite inject boolean");
-                                    errors++;
-                                } else {
-                                    if(strncmp((char*)sub_event.data.scalar.value, "yes", 3) == 0 ||
-                                       strncmp((char*)sub_event.data.scalar.value, "true", 4) == 0)
-                                        flags |= RW_INJECT;
-                                    else
-                                        flags &= ~RW_INJECT;
-
-                                    yaml_event_delete(&sub_event);
-                                }
-                            } else {
-                                yaml_error(parser, &sub_event, "Unexpected scalar in rewrite mapping");
-                                errors++;
-                            }
-                            break;
-
-                        case YAML_MAPPING_END_EVENT:
-                            if(key) {
-                                if (!log_job_rewrite_add(jb, key, flags, search_pattern, replace_pattern))
-                                    errors++;
-                            }
-
-                            freez(key);
-                            key = NULL;
-
-                            freez(search_pattern);
-                            search_pattern = NULL;
-
-                            freez(replace_pattern);
-                            replace_pattern = NULL;
-
-                            flags = RW_NONE;
-
-                            mapping_finished = true;
-                            break;
-
-                        default:
-                            yaml_error(parser, &sub_event, "Unexpected event in rewrite mapping");
-                            errors++;
-                            break;
-                    }
+                    handle_rewrite_event(parser, &sub_event, &key,
+                                         &search_pattern, &replace_pattern,
+                                         &flags, &mapping_finished, jb, &errors);
 
                     yaml_event_delete(&sub_event);
                 }
-                freez(replace_pattern);
-                replace_pattern = NULL;
-                freez(search_pattern);
-                search_pattern = NULL;
-                freez(key);
-                key = NULL;
-            }
                 break;
+            }
 
             case YAML_SEQUENCE_END_EVENT:
                 finished = true;
@@ -618,25 +657,30 @@ static size_t yaml_parse_renames(yaml_parser_t *parser, LOG_JOB *jb) {
                     switch (sub_event.type) {
                         case YAML_SCALAR_EVENT:
                             if (yaml_scalar_matches(&sub_event, "new_key", strlen("new_key"))) {
-                                if (!yaml_parse(parser, &sub_event) || sub_event.type != YAML_SCALAR_EVENT) {
-                                    yaml_error(parser, &sub_event, "Expected scalar for rename new_key");
+                                yaml_event_t value_event;
+
+                                if (!yaml_parse(parser, &value_event) || value_event.type != YAML_SCALAR_EVENT) {
+                                    yaml_error(parser, &value_event, "Expected scalar for rename new_key");
                                     errors++;
                                 } else {
-                                    hashed_key_len_set(&rn.new_key, (char *)sub_event.data.scalar.value, sub_event.data.scalar.length);
-                                    yaml_event_delete(&sub_event);
+                                    hashed_key_len_set(&rn.new_key, (char *)value_event.data.scalar.value, value_event.data.scalar.length);
+                                    yaml_event_delete(&value_event);
                                 }
                             } else if (yaml_scalar_matches(&sub_event, "old_key", strlen("old_key"))) {
-                                if (!yaml_parse(parser, &sub_event) || sub_event.type != YAML_SCALAR_EVENT) {
-                                    yaml_error(parser, &sub_event, "Expected scalar for rename old_key");
+                                yaml_event_t value_event;
+
+                                if (!yaml_parse(parser, &value_event) || value_event.type != YAML_SCALAR_EVENT) {
+                                    yaml_error(parser, &value_event, "Expected scalar for rename old_key");
                                     errors++;
                                 } else {
-                                    hashed_key_len_set(&rn.old_key, (char *)sub_event.data.scalar.value, sub_event.data.scalar.length);
-                                    yaml_event_delete(&sub_event);
+                                    hashed_key_len_set(&rn.old_key, (char *)value_event.data.scalar.value, value_event.data.scalar.length);
+                                    yaml_event_delete(&value_event);
                                 }
                             } else {
                                 yaml_error(parser, &sub_event, "Unexpected scalar in rewrite mapping");
                                 errors++;
                             }
+
                             break;
 
                         case YAML_MAPPING_END_EVENT:
@@ -793,7 +837,11 @@ bool yaml_parse_file(const char *config_file_path, LOG_JOB *jb) {
     }
 
     yaml_parser_t parser;
-    yaml_parser_initialize(&parser);
+    if (!yaml_parser_initialize(&parser)) {
+        fclose(fp);
+        return false;
+    }
+
     yaml_parser_set_input_file(&parser, fp);
 
     size_t errors = yaml_parse_initialized(&parser, jb);

src/collectors/log2journal/log2journal.h

@@ -245,7 +245,7 @@ typedef enum __attribute__((__packed__)) {
 
     HK_HASHTABLE_ALLOCATED  = (1 << 0), // this is key object allocated in the hashtable
                                         // objects that do not have this, have a pointer to a key in the hashtable
-                                        // objects that have this, value a value allocated
+                                        // objects that have this, value is allocated
 
     HK_FILTERED             = (1 << 1), // we checked once if this key in filtered
     HK_FILTERED_INCLUDED    = (1 << 2), // the result of the filtering was to include it in the output
@@ -274,15 +274,16 @@ typedef struct hashed_key {
 } HASHED_KEY;
 
 static inline void hashed_key_cleanup(HASHED_KEY *k) {
-    if(k->key) {
-        freez((void *)k->key);
-        k->key = NULL;
-    }
-
     if(k->flags & HK_HASHTABLE_ALLOCATED)
         txt_cleanup(&k->value);
     else
         k->hashtable_ptr = NULL;
+
+    freez((void *)k->key);
+    k->key = NULL;
+    k->len = 0;
+    k->hash = 0;
+    k->flags = HK_NONE;
 }
 
 static inline void hashed_key_set(HASHED_KEY *k, const char *name) {