field-cache: zero-initialise cache in shearwater and garmin parser create

dc_parser_allocate() uses malloc, leaving embedded struct members
uninitialised. dc_field_cache_free() is now called from the destroy
path of both parsers before the cache is ever populated; it dereferences
strings[].value, which would be garbage pointers.

Add memset(&parser->cache, 0, sizeof(parser->cache)) in
shearwater_common_parser_create() and garmin_parser_create() immediately
after dc_parser_allocate() succeeds, making all destroy and pre-reset
free calls safe.

Signed-off-by: Subsurface CI <ci@subsurface-divelog.org>

Author: mikeller

src/field-cache.c

@@ -1,5 +1,6 @@
 #include <stdio.h>
 #include <stdarg.h>
+#include <stdlib.h>
 #include <string.h>
 
 #include "platform.h"
@@ -119,3 +120,16 @@ dc_field_get(dc_field_cache_t *cache, dc_field_type_t type, unsigned int flags,
 
 	return DC_STATUS_UNSUPPORTED;
 }
+
+/*
+ * Free all strdup'd string values in the cache.  The desc pointers are
+ * static allocations and must not be freed.  Safe to call on a
+ * zero-initialised cache (free(NULL) is a no-op).
+ */
+void dc_field_cache_free(dc_field_cache_t *cache)
+{
+	for (int i = 0; i < MAXSTRINGS; i++) {
+		free((void *) cache->strings[i].value);
+		cache->strings[i].value = NULL;
+	}
+}

src/field-cache.h

@@ -39,6 +39,7 @@ dc_status_t dc_field_add_string(dc_field_cache_t *, const char *desc, const char
 dc_status_t dc_field_add_string_fmt(dc_field_cache_t *, const char *desc, const char *fmt, ...);
 dc_status_t dc_field_get_string(dc_field_cache_t *, unsigned idx, dc_field_string_t *value);
 dc_status_t dc_field_get(dc_field_cache_t *, dc_field_type_t, unsigned int, void *);
+void dc_field_cache_free(dc_field_cache_t *);
 
 /*
  * Macro to make it easy to set DC_FIELD_xyz values.

src/garmin_parser.c

@@ -327,6 +327,7 @@ static dc_status_t garmin_parser_set_data (garmin_parser_t *garmin, const unsign
 static dc_status_t garmin_parser_get_datetime (dc_parser_t *abstract, dc_datetime_t *datetime);
 static dc_status_t garmin_parser_get_field (dc_parser_t *abstract, dc_field_type_t type, unsigned int flags, void *value);
 static dc_status_t garmin_parser_samples_foreach (dc_parser_t *abstract, dc_sample_callback_t callback, void *userdata);
+static dc_status_t garmin_parser_destroy (dc_parser_t *abstract);
 
 static const dc_parser_vtable_t garmin_parser_vtable = {
 	sizeof(garmin_parser_t),
@@ -337,7 +338,7 @@ static const dc_parser_vtable_t garmin_parser_vtable = {
 	garmin_parser_get_datetime, /* datetime */
 	garmin_parser_get_field, /* fields */
 	garmin_parser_samples_foreach, /* samples_foreach */
-	NULL /* destroy */
+	garmin_parser_destroy /* destroy */
 };
 
 dc_status_t
@@ -355,6 +356,8 @@ garmin_parser_create (dc_parser_t **out, dc_context_t *context, const unsigned c
 		return DC_STATUS_NOMEMORY;
 	}
 
+	memset(&parser->cache, 0, sizeof(parser->cache));
+
 	garmin_parser_set_data(parser, data, size);
 
 	*out = (dc_parser_t *) parser;
@@ -1793,3 +1796,13 @@ garmin_parser_samples_foreach (dc_parser_t *abstract, dc_sample_callback_t callb
 	garmin->userdata = userdata;
 	return traverse_data(garmin);
 }
+
+static dc_status_t
+garmin_parser_destroy (dc_parser_t *abstract)
+{
+	garmin_parser_t *garmin = (garmin_parser_t *) abstract;
+
+	dc_field_cache_free(&garmin->cache);
+
+	return DC_STATUS_SUCCESS;
+}

src/shearwater_predator_parser.c

@@ -194,6 +194,7 @@ struct dc_parser_sensor_calibration_t {
 static dc_status_t shearwater_predator_parser_get_datetime (dc_parser_t *abstract, dc_datetime_t *datetime);
 static dc_status_t shearwater_predator_parser_get_field (dc_parser_t *abstract, dc_field_type_t type, unsigned int flags, void *value);
 static dc_status_t shearwater_predator_parser_samples_foreach (dc_parser_t *abstract, dc_sample_callback_t callback, void *userdata);
+static dc_status_t shearwater_predator_parser_destroy (dc_parser_t *abstract);
 
 static dc_status_t shearwater_predator_parser_cache (shearwater_predator_parser_t *parser);
 
@@ -206,7 +207,7 @@ static const dc_parser_vtable_t shearwater_predator_parser_vtable = {
 	shearwater_predator_parser_get_datetime, /* datetime */
 	shearwater_predator_parser_get_field, /* fields */
 	shearwater_predator_parser_samples_foreach, /* samples_foreach */
-	NULL /* destroy */
+	shearwater_predator_parser_destroy /* destroy */
 };
 
 static const dc_parser_vtable_t shearwater_petrel_parser_vtable = {
@@ -218,7 +219,7 @@ static const dc_parser_vtable_t shearwater_petrel_parser_vtable = {
 	shearwater_predator_parser_get_datetime, /* datetime */
 	shearwater_predator_parser_get_field, /* fields */
 	shearwater_predator_parser_samples_foreach, /* samples_foreach */
-	NULL /* destroy */
+	shearwater_predator_parser_destroy /* destroy */
 };
 
 
@@ -267,6 +268,8 @@ shearwater_common_parser_create (dc_parser_t **out, dc_context_t *context, const
 		return DC_STATUS_NOMEMORY;
 	}
 
+	memset(&parser->cache, 0, sizeof(parser->cache));
+
 	// Set the default values.
 	parser->model = model;
 	parser->petrel = petrel;
@@ -464,6 +467,16 @@ static void add_sensor_state(shearwater_predator_parser_t *parser, bool external
 	}
 }
 
+static dc_status_t
+shearwater_predator_parser_destroy (dc_parser_t *abstract)
+{
+	shearwater_predator_parser_t *parser = (shearwater_predator_parser_t *) abstract;
+
+	dc_field_cache_free(&parser->cache);
+
+	return DC_STATUS_SUCCESS;
+}
+
 static dc_status_t
 shearwater_predator_parser_cache (shearwater_predator_parser_t *parser)
 {
@@ -474,6 +487,7 @@ shearwater_predator_parser_cache (shearwater_predator_parser_t *parser)
 	if (parser->cached) {
 		return DC_STATUS_SUCCESS;
 	}
+	dc_field_cache_free(&parser->cache);
 	memset(&parser->cache, 0, sizeof(parser->cache));
 
 	// Log versions before 6 weren't reliably stored in the data, but