From b0f7bb3495f72e0c5d8b1fe792a5731554cc1172 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Tue, 19 May 2026 15:03:36 +0200 Subject: [PATCH 1/4] remove unused `cfg_map_add()` function Function `cfg_map_add()` was unused, it is now removed. --- lib/isccfg/include/isccfg/cfg.h | 11 ----------- lib/isccfg/parser.c | 17 ----------------- 2 files changed, 28 deletions(-) diff --git a/lib/isccfg/include/isccfg/cfg.h b/lib/isccfg/include/isccfg/cfg.h index 5012f49217..592f538cd0 100644 --- a/lib/isccfg/include/isccfg/cfg.h +++ b/lib/isccfg/include/isccfg/cfg.h @@ -219,17 +219,6 @@ cfg_map_get(const cfg_obj_t *mapobj, const char *name, const cfg_obj_t **obj); * \li #ISC_R_NOTFOUND - name not found in map */ -isc_result_t -cfg_map_add(cfg_obj_t *mapobj, cfg_obj_t *obj, const char *clause); -/*%< - * Add the object 'obj' to the specified clause in mapbody 'mapobj'. - * Used for adding new zones. - * - * Require: - * \li 'obj' is a valid cfg_obj_t. - * \li 'mapobj' is a valid cfg_obj_t of type map. - */ - const cfg_obj_t * cfg_map_getname(const cfg_obj_t *mapobj); /*%< diff --git a/lib/isccfg/parser.c b/lib/isccfg/parser.c index 83722cb858..c19239e37c 100644 --- a/lib/isccfg/parser.c +++ b/lib/isccfg/parser.c @@ -4042,23 +4042,6 @@ map_define(cfg_obj_t *mapobj, cfg_obj_t *obj, const cfg_clausedef_t *clause) { return result; } -isc_result_t -cfg_map_add(cfg_obj_t *mapobj, cfg_obj_t *obj, const char *clausename) { - const cfg_clausedef_t *clause; - - REQUIRE(VALID_CFGOBJ(obj)); - REQUIRE(VALID_CFGOBJ(mapobj)); - REQUIRE(mapobj->type->rep == &cfg_rep_map); - REQUIRE(clausename != NULL); - - clause = cfg_map_findclause(mapobj->type, clausename); - if (clause == NULL || clause->name == NULL) { - return ISC_R_FAILURE; - } - - return map_define(mapobj, obj, clause); -} - isc_result_t cfg_map_addclone(cfg_obj_t *map, const cfg_obj_t *obj, const cfg_clausedef_t *clause) { From d1c55d78c63dc7a32ce2bfd134711e1f710aa528 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Tue, 19 May 2026 15:04:11 +0200 Subject: [PATCH 2/4] fix possible NULL dereference in `cfg_map_findclause()` `cfg_map_findclause()` did not check whether a clause existed before dereferencing it, which could lead to a NULL dereference. Add the missing check to prevent this. In practice, this was not triggering any known bug, since `cfg_map_findclause()` is only called in contexts where the clause is known to exist. --- lib/isccfg/parser.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/isccfg/parser.c b/lib/isccfg/parser.c index c19239e37c..80ceeadb81 100644 --- a/lib/isccfg/parser.c +++ b/lib/isccfg/parser.c @@ -2948,10 +2948,14 @@ cfg_map_findclause(const cfg_type_t *map, const char *name) { REQUIRE(name != NULL); found = cfg_map_firstclause(map, &clauses, &idx); - while (name != NULL && strcasecmp(name, found->name)) { + while (found != NULL && name != NULL && strcasecmp(name, found->name)) { found = cfg_map_nextclause(map, &clauses, &idx); } + if (found == NULL) { + return found; + } + return ((cfg_clausedef_t *)clauses) + idx; } From b13bd6cb8519d190cfe065278bfba799790101f8 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Tue, 19 May 2026 15:12:36 +0200 Subject: [PATCH 3/4] add test for `cfg_map_findclause()` Add a test for `cfg_map_findclause()` ensuring its correct behaviour (returning NULL) if a clause does not exists. --- tests/isccfg/parser_test.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/isccfg/parser_test.c b/tests/isccfg/parser_test.c index 25d46ff7e7..4e30da35a6 100644 --- a/tests/isccfg/parser_test.c +++ b/tests/isccfg/parser_test.c @@ -303,6 +303,18 @@ view \"_bind\" chaos {\n\ cfg_obj_detach(&clone); } +static const cfg_clausedef_t *const empty_clausesets[] = { NULL }; + +static cfg_type_t cfg_type_empty_map = { + "empty_map", NULL, NULL, NULL, &cfg_rep_map, &empty_clausesets, +}; + +ISC_RUN_TEST_IMPL(cfg_map_findclause_empty) { + const cfg_clausedef_t *result = cfg_map_findclause(&cfg_type_empty_map, + "anything"); + assert_null(result); +} + ISC_TEST_LIST_START ISC_TEST_ENTRY(addzoneconf) @@ -310,6 +322,7 @@ ISC_TEST_ENTRY(parse_buffer) ISC_TEST_ENTRY(cfg_map_firstclause) ISC_TEST_ENTRY(cfg_map_nextclause) ISC_TEST_ENTRY(cfg_clone_copy) +ISC_TEST_ENTRY(cfg_map_findclause_empty) ISC_TEST_LIST_END From 3594b0ff7a3256ae05c0edeaa182f8cd380723b4 Mon Sep 17 00:00:00 2001 From: Colin Vidal Date: Thu, 21 May 2026 09:53:59 +0200 Subject: [PATCH 4/4] renamed `cfg_map_addclone()` into `cfg_map_add()` Since there is no `cfg_map_add()` anymore, and `cfg_map_addclone()` wasn't quite clear, let's rename to `cfg_map_addclone()` into `cfg_map_add()`, as this is fundamentally what this function is doing. --- lib/isccfg/include/isccfg/cfg.h | 6 +++--- lib/isccfg/namedconf.c | 6 +++--- lib/isccfg/parser.c | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/isccfg/include/isccfg/cfg.h b/lib/isccfg/include/isccfg/cfg.h index 592f538cd0..73c7a926eb 100644 --- a/lib/isccfg/include/isccfg/cfg.h +++ b/lib/isccfg/include/isccfg/cfg.h @@ -136,10 +136,10 @@ cfg_parser_currentfile(cfg_parser_t *pctx); */ isc_result_t -cfg_map_addclone(cfg_obj_t *map, const cfg_obj_t *obj, - const cfg_clausedef_t *clause); +cfg_map_add(cfg_obj_t *map, const cfg_obj_t *obj, + const cfg_clausedef_t *clause); /*%< - * Add a clone of 'obj' to the specified clause in mapbody 'mapobj'. + * Clone 'obj' and add its clone to the specified clause in mapbody 'mapobj'. * If the clause is tagged with CFG_CLAUSEFLAG_MULTI, the function expects * that 'obj' is a list and will clone each element and sequentially add them * (preserving the order), instead of adding a list as single element of diff --git a/lib/isccfg/namedconf.c b/lib/isccfg/namedconf.c index fbccca2643..ea3b18aae6 100644 --- a/lib/isccfg/namedconf.c +++ b/lib/isccfg/namedconf.c @@ -1176,8 +1176,8 @@ map_merge(const cfg_obj_t *config ISC_ATTR_UNUSED, cfg_obj_t *effectivemap, if (effectiveres == ISC_R_NOTFOUND && defaultres == ISC_R_SUCCESS) { - INSIST(cfg_map_addclone(effectivemap, defaultobj, - clause) == ISC_R_SUCCESS); + INSIST(cfg_map_add(effectivemap, defaultobj, clause) == + ISC_R_SUCCESS); continue; } @@ -1205,7 +1205,7 @@ cloneto(cfg_obj_t *options, const cfg_obj_t *obj, const char *clausename) { const cfg_clausedef_t *clause = cfg_map_findclause(options->type, clausename); - result = cfg_map_addclone(options, obj, clause); + result = cfg_map_add(options, obj, clause); INSIST(result == ISC_R_SUCCESS); } diff --git a/lib/isccfg/parser.c b/lib/isccfg/parser.c index 80ceeadb81..854f0b9f95 100644 --- a/lib/isccfg/parser.c +++ b/lib/isccfg/parser.c @@ -4047,8 +4047,8 @@ map_define(cfg_obj_t *mapobj, cfg_obj_t *obj, const cfg_clausedef_t *clause) { } isc_result_t -cfg_map_addclone(cfg_obj_t *map, const cfg_obj_t *obj, - const cfg_clausedef_t *clause) { +cfg_map_add(cfg_obj_t *map, const cfg_obj_t *obj, + const cfg_clausedef_t *clause) { isc_result_t result = ISC_R_SUCCESS; cfg_obj_t *clone = NULL;