[Fix] Fix usage of unsafe ucl iterators

6 files changed, 93 insertions, 21 deletions

diff --git a/src/controller.c b/src/controller.c
index 9733368a2..8949da733 100644
--- a/src/controller.c
+++ b/src/controller.c
@@ -2001,8 +2001,10 @@ rspamd_controller_handle_saveactions (
 		return 0;
 	}
 
+	it = ucl_object_iterate_new (obj);
+
 	for (i = 0; i < 3; i++) {
-		cur = ucl_object_iterate (obj, &it, TRUE);
+		cur = ucl_object_iterate_safe (it, TRUE);
 		if (cur == NULL) {
 			break;
 		}
@@ -2029,6 +2031,8 @@ rspamd_controller_handle_saveactions (
 		}
 	}
 
+	ucl_object_iterate_free (it);
+
 	if (dump_dynamic_config (ctx->cfg)) {
 		msg_info_session ("<%s> modified %d actions",
 			rspamd_inet_address_to_string (session->from_addr),
@@ -2111,11 +2115,15 @@ rspamd_controller_handle_savesymbols (
 		return 0;
 	}
 
-	while ((cur = ucl_object_iterate (obj, &iter, true))) {
+	iter = ucl_object_iterate_new (obj);
+
+	while ((cur = ucl_object_iterate_safe (iter, true))) {
 		if (cur->type != UCL_OBJECT) {
 			msg_err_session ("json array data error");
 			rspamd_controller_send_error (conn_ent, 400, "Cannot parse input");
 			ucl_object_unref (obj);
+			ucl_object_iterate_free (iter);
+
 			return 0;
 		}
 
@@ -2132,6 +2140,8 @@ rspamd_controller_handle_savesymbols (
 				rspamd_controller_send_error (conn_ent, 506,
 					"Add symbol failed");
 				ucl_object_unref (obj);
+				ucl_object_iterate_free (iter);
+
 				return 0;
 			}
 			added ++;
@@ -2144,6 +2154,8 @@ rspamd_controller_handle_savesymbols (
 		}
 	}
 
+	ucl_object_iterate_free (iter);
+
 	if (added > 0) {
 		if (ctx->cfg->dynamic_conf) {
 			if (dump_dynamic_config (ctx->cfg)) {
diff --git a/src/fuzzy_storage.c b/src/fuzzy_storage.c
index e0a31a9a4..a1eb80997 100644
--- a/src/fuzzy_storage.c
+++ b/src/fuzzy_storage.c
@@ -1985,7 +1985,7 @@ fuzzy_parse_keypair (rspamd_mempool_t *pool,
 	else if (ucl_object_type (obj) == UCL_ARRAY) {
 		while ((cur = ucl_object_iterate (obj, &it, true)) != NULL) {
 			if (!fuzzy_parse_keypair (pool, cur, pd, section, err)) {
-				return FALSE;
+				msg_err_pool ("cannot parse keypair");
 			}
 		}
 	}
diff --git a/src/libserver/cfg_rcl.c b/src/libserver/cfg_rcl.c
index 8655ee8de..9d1698c22 100644
--- a/src/libserver/cfg_rcl.c
+++ b/src/libserver/cfg_rcl.c
@@ -420,14 +420,15 @@ rspamd_rcl_actions_handler (rspamd_mempool_t *pool, const ucl_object_t *obj,
 	struct metric_actions_cbdata *cbdata = ud;
 	gint action_value;
 	const ucl_object_t *cur;
-	ucl_object_iter_t it = NULL;
+	ucl_object_iter_t it;
 	struct rspamd_metric *metric;
 	struct rspamd_config *cfg;
 
 	metric = cbdata->metric;
 	cfg = cbdata->cfg;
+	it = ucl_object_iterate_new (obj);
 
-	while ((cur = ucl_object_iterate (obj, &it, true)) != NULL) {
+	while ((cur = ucl_object_iterate_safe (it, true)) != NULL) {
 		if (!rspamd_action_from_str (ucl_object_key (cur), &action_value) ||
 				!ucl_object_todouble_safe (cur, &action_score)) {
 			g_set_error (err,
@@ -435,6 +436,8 @@ rspamd_rcl_actions_handler (rspamd_mempool_t *pool, const ucl_object_t *obj,
 					EINVAL,
 					"invalid action definition: '%s'",
 					ucl_object_key (cur));
+			ucl_object_iterate_free (it);
+
 			return FALSE;
 		}
 		else {
@@ -444,6 +447,8 @@ rspamd_rcl_actions_handler (rspamd_mempool_t *pool, const ucl_object_t *obj,
 		}
 	}
 
+	ucl_object_iterate_free (it);
+
 	return TRUE;
 }
 
@@ -604,6 +609,7 @@ rspamd_rcl_worker_handler (rspamd_mempool_t *pool, const ucl_object_t *obj,
 	/* This name is more logical */
 	if (val != NULL) {
 		it = ucl_object_iterate_new (val);
+
 		while ((cur = ucl_object_iterate_safe (it, true)) != NULL) {
 			if (!ucl_object_tostring_safe (cur, &worker_bind)) {
 				continue;
@@ -618,6 +624,7 @@ rspamd_rcl_worker_handler (rspamd_mempool_t *pool, const ucl_object_t *obj,
 				return FALSE;
 			}
 		}
+
 		ucl_object_iterate_free (it);
 	}
 
@@ -632,8 +639,9 @@ rspamd_rcl_worker_handler (rspamd_mempool_t *pool, const ucl_object_t *obj,
 	wparser = g_hash_table_lookup (cfg->wrk_parsers, &qtype);
 
 	if (wparser != NULL && obj->type == UCL_OBJECT) {
-		it = NULL;
-		while ((cur = ucl_object_iterate (obj, &it, true)) != NULL) {
+		it = ucl_object_iterate_new (obj);
+
+		while ((cur = ucl_object_iterate_safe (it, true)) != NULL) {
 			srch.name = ucl_object_key (cur);
 			srch.ptr = wrk->ctx; /* XXX: is it valid? */
 			whandler = g_hash_table_lookup (wparser->parsers, &srch);
@@ -646,6 +654,8 @@ rspamd_rcl_worker_handler (rspamd_mempool_t *pool, const ucl_object_t *obj,
 							&whandler->parser,
 							section,
 							err)) {
+
+						ucl_object_iterate_free (it);
 						return FALSE;
 					}
 
@@ -655,6 +665,9 @@ rspamd_rcl_worker_handler (rspamd_mempool_t *pool, const ucl_object_t *obj,
 				}
 			}
 		}
+
+		ucl_object_iterate_free (it);
+
 		if (wparser->def_obj_parser != NULL) {
 			robj = ucl_object_ref (obj);
 
@@ -1244,8 +1257,11 @@ rspamd_rcl_classifier_handler (rspamd_mempool_t *pool,
 			ccf->name = ccf->classifier;
 		}
 
-		while ((val = ucl_object_iterate (obj, &it, true)) != NULL && res) {
+		it = ucl_object_iterate_new (obj);
+
+		while ((val = ucl_object_iterate_safe (it, true)) != NULL && res) {
 			st_key = ucl_object_key (val);
+
 			if (st_key != NULL) {
 				if (g_ascii_strcasecmp (st_key, "statfile") == 0) {
 					LL_FOREACH (val, cur) {
@@ -1255,6 +1271,8 @@ rspamd_rcl_classifier_handler (rspamd_mempool_t *pool,
 								cur, cfg->cfg_pool, err);
 
 						if (!res) {
+							ucl_object_iterate_free (it);
+
 							return FALSE;
 						}
 					}
@@ -1282,6 +1300,8 @@ rspamd_rcl_classifier_handler (rspamd_mempool_t *pool,
 				}
 			}
 		}
+
+		ucl_object_iterate_free (it);
 	}
 	else {
 		msg_err_config ("fatal configuration error, cannot parse statfile definition");
@@ -1493,13 +1513,18 @@ rspamd_rcl_composites_handler (rspamd_mempool_t *pool,
 	const ucl_object_t *cur;
 	gboolean success = TRUE;
 
-	while ((cur = ucl_iterate_object (obj, &it, true))) {
-		success = rspamd_rcl_composite_handler(pool, cur, ucl_object_key(cur), ud, section, err);
+	it = ucl_object_iterate_new (obj);
+
+	while ((cur = ucl_object_iterate_safe (it, true))) {
+		success = rspamd_rcl_composite_handler(pool, cur,
+				ucl_object_key(cur), ud, section, err);
 		if (!success) {
 			break;
 		}
 	}
 
+	ucl_object_iterate_free (it);
+
 	return success;
 }
 
@@ -2487,15 +2512,17 @@ rspamd_rcl_process_section (struct rspamd_config *cfg,
 	g_assert (obj != NULL);
 	g_assert (sec->handler != NULL);
 
-	it = NULL;
-
 	if (sec->key_attr != NULL) {
-		while ((cur = ucl_object_iterate (obj, &it, true)) != NULL) {
+		it = ucl_object_iterate_new (obj);
+
+		while ((cur = ucl_object_iterate_full (it, UCL_ITERATE_EXPLICIT)) != NULL) {
 			if (ucl_object_type (cur) != UCL_OBJECT) {
 				is_nested = FALSE;
 				break;
 			}
 		}
+
+		ucl_object_iterate_free (it);
 	}
 	else {
 		is_nested = FALSE;
@@ -2503,14 +2530,18 @@ rspamd_rcl_process_section (struct rspamd_config *cfg,
 
 	if (is_nested) {
 		/* Just reiterate on all subobjects */
-		it = NULL;
+		it = ucl_object_iterate_new (obj);
 
-		while ((cur = ucl_object_iterate (obj, &it, true)) != NULL) {
+		while ((cur = ucl_object_iterate_full (it, UCL_ITERATE_EXPLICIT)) != NULL) {
 			if (!sec->handler (pool, cur, ucl_object_key (cur), ptr, sec, err)) {
+				ucl_object_iterate_free (it);
+
 				return FALSE;
 			}
 		}
 
+		ucl_object_iterate_free (it);
+
 		return TRUE;
 	}
 	else {
@@ -2521,8 +2552,10 @@ rspamd_rcl_process_section (struct rspamd_config *cfg,
 			if (cur == NULL) {
 				if (sec->default_key == NULL) {
 					g_set_error (err, CFG_RCL_ERROR, EINVAL, "required attribute "
-							"'%s' is missing for section '%s'", sec->key_attr,
-							sec->name);
+							"'%s' is missing for section '%s', current key: %s",
+							sec->key_attr,
+							sec->name,
+							ucl_object_emit (obj, UCL_EMIT_CONFIG));
 					return FALSE;
 				}
 				else {
@@ -3054,12 +3087,16 @@ rspamd_rcl_parse_struct_string_list (rspamd_mempool_t *pool,
 					EINVAL,
 					"cannot convert an object or array to string: %s",
 					ucl_object_key (obj));
+			ucl_object_iterate_free (iter);
+
 			return FALSE;
 		}
 
 		rspamd_rcl_insert_string_list_item (target, pool, val, is_hash);
 	}
 
+	ucl_object_iterate_free (iter);
+
 	if (*target == NULL) {
 		g_set_error (err,
 				CFG_RCL_ERROR,
diff --git a/src/libserver/cfg_utils.c b/src/libserver/cfg_utils.c
index 8ee284212..606ef3cf7 100644
--- a/src/libserver/cfg_utils.c
+++ b/src/libserver/cfg_utils.c
@@ -1698,7 +1698,9 @@ rspamd_config_radix_from_ucl (struct rspamd_config *cfg,
 			break;
 		case UCL_ARRAY:
 			/* List of IP addresses */
-			while ((cur = ucl_iterate_object (cur_elt, &it, true)) != NULL) {
+			it = ucl_object_iterate_new (cur_elt);
+
+			while ((cur = ucl_object_iterate_safe (it, true)) != NULL) {
 				str = ucl_object_tostring (cur);
 
 				if (str == NULL || !radix_add_generic_iplist (str, target, TRUE)) {
@@ -1710,9 +1712,13 @@ rspamd_config_radix_from_ucl (struct rspamd_config *cfg,
 						radix_destroy_compressed (*target);
 					}
 
+					ucl_object_iterate_free (it);
+
 					return FALSE;
 				}
 			}
+
+			ucl_object_iterate_free (it);
 			break;
 		default:
 			g_set_error (err, g_quark_from_static_string ("rspamd-config"),
diff --git a/src/libserver/dynamic_cfg.c b/src/libserver/dynamic_cfg.c
index b74b0a8f9..6e319ed36 100644
--- a/src/libserver/dynamic_cfg.c
+++ b/src/libserver/dynamic_cfg.c
@@ -346,16 +346,22 @@ dynamic_metric_find_elt (const ucl_object_t *arr, const gchar *name)
 	ucl_object_iter_t it = NULL;
 	const ucl_object_t *cur, *n;
 
-	while ((cur = ucl_object_iterate (arr, &it, true)) != NULL) {
+	it = ucl_object_iterate_new (arr);
+
+	while ((cur = ucl_object_iterate_safe (it, true)) != NULL) {
 		if (cur->type == UCL_OBJECT) {
 			n = ucl_object_lookup (cur, "name");
 			if (n && n->type == UCL_STRING &&
 				strcmp (name, ucl_object_tostring (n)) == 0) {
+				ucl_object_iterate_free (it);
+
 				return (ucl_object_t *)ucl_object_lookup (cur, "value");
 			}
 		}
 	}
 
+	ucl_object_iterate_free (it);
+
 	return NULL;
 }
 
@@ -365,16 +371,22 @@ dynamic_metric_find_metric (const ucl_object_t *arr, const gchar *metric)
 	ucl_object_iter_t it = NULL;
 	const ucl_object_t *cur, *n;
 
-	while ((cur = ucl_object_iterate (arr, &it, true)) != NULL) {
+	it = ucl_object_iterate_new (arr);
+
+	while ((cur = ucl_object_iterate_safe (it, true)) != NULL) {
 		if (cur->type == UCL_OBJECT) {
 			n = ucl_object_lookup (cur, "metric");
 			if (n && n->type == UCL_STRING &&
 				strcmp (metric, ucl_object_tostring (n)) == 0) {
+				ucl_object_iterate_free (it);
+
 				return (ucl_object_t *)cur;
 			}
 		}
 	}
 
+	ucl_object_iterate_free (it);
+
 	return NULL;
 }
 
diff --git a/src/libutil/map.c b/src/libutil/map.c
index 3a7cdc032..361abcee4 100644
--- a/src/libutil/map.c
+++ b/src/libutil/map.c
@@ -1628,7 +1628,9 @@ rspamd_map_add_from_ucl (struct rspamd_config *cfg,
 
 		if (ucl_object_type (elt) == UCL_ARRAY) {
 			/* Add array of maps as multiple backends */
-			while ((cur = ucl_object_iterate (elt, &it, true)) != NULL) {
+			it = ucl_object_iterate_new (elt);
+
+			while ((cur = ucl_object_iterate_safe (it, true)) != NULL) {
 				if (ucl_object_type (cur) == UCL_STRING) {
 					bk = rspamd_map_parse_backend (cfg, ucl_object_tostring (cur));
 
@@ -1643,10 +1645,13 @@ rspamd_map_add_from_ucl (struct rspamd_config *cfg,
 				else {
 					msg_err_config ("bad map element type: %s",
 							ucl_object_type_to_string (ucl_object_type (cur)));
+					ucl_object_iterate_free (it);
 					goto err;
 				}
 			}
 
+			ucl_object_iterate_free (it);
+
 			if (map->backends->len == 0) {
 				msg_err_config ("map has no urls to be loaded: empty object list");
 				goto err;