diff --git a/CHANGES b/CHANGES index ac25049330..bd5aa97be4 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +3910. [bug] When computing the number of elements required for a + acl count_acl_elements could have a short count leading + to a assertion failure. Also zero out new acl elements + in dns_acl_merge. [RT #36675] + 3908. [bug] rndc now differentiates between a zone in multiple views and a zone that doesn't exist at all. [RT #36691] diff --git a/lib/dns/acl.c b/lib/dns/acl.c index 64cc08a792..880fc3648e 100644 --- a/lib/dns/acl.c +++ b/lib/dns/acl.c @@ -291,6 +291,9 @@ dns_acl_merge(dns_acl_t *dest, dns_acl_t *source, isc_boolean_t pos) if (newmem == NULL) return (ISC_R_NOMEMORY); + /* Zero. */ + memset(newmem, 0, newalloc * sizeof(dns_aclelement_t)); + /* Copy in the original elements */ memmove(newmem, dest->elements, dest->length * sizeof(dns_aclelement_t)); diff --git a/lib/isccfg/aclconf.c b/lib/isccfg/aclconf.c index 8b8035877e..76f6ad4a9a 100644 --- a/lib/isccfg/aclconf.c +++ b/lib/isccfg/aclconf.c @@ -222,14 +222,16 @@ convert_keyname(const cfg_obj_t *keyobj, isc_log_t *lctx, isc_mem_t *mctx, * elements table after all the nested ACLs have been merged in to the * parent. */ -static int +static isc_result_t count_acl_elements(const cfg_obj_t *caml, const cfg_obj_t *cctx, - isc_boolean_t *has_negative) + isc_log_t *lctx, cfg_aclconfctx_t *ctx, isc_mem_t *mctx, + isc_uint32_t *count, isc_boolean_t *has_negative) { const cfg_listelt_t *elt; - const cfg_obj_t *cacl = NULL; isc_result_t result; - int n = 0; + isc_uint32_t n = 0; + + REQUIRE(count != NULL); if (has_negative != NULL) *has_negative = ISC_FALSE; @@ -254,14 +256,19 @@ count_acl_elements(const cfg_obj_t *caml, const cfg_obj_t *cctx, n++; } else if (cfg_obj_islist(ce)) { isc_boolean_t negative; - n += count_acl_elements(ce, cctx, &negative); + isc_uint32_t sub; + result = count_acl_elements(ce, cctx, lctx, ctx, mctx, + &sub, &negative); + if (result != ISC_R_SUCCESS) + return (result); + n += sub; if (negative) n++; #ifdef HAVE_GEOIP } else if (cfg_obj_istuple(ce) && cfg_obj_isvoid(cfg_tuple_get(ce, "negated"))) { - n++; + n++; #endif /* HAVE_GEOIP */ } else if (cfg_obj_isstring(ce)) { const char *name = cfg_obj_asstring(ce); @@ -270,15 +277,27 @@ count_acl_elements(const cfg_obj_t *caml, const cfg_obj_t *cctx, n++; } else if (strcasecmp(name, "any") != 0 && strcasecmp(name, "none") != 0) { - result = get_acl_def(cctx, name, &cacl); - if (result == ISC_R_SUCCESS) - n += count_acl_elements(cacl, cctx, - NULL) + 1; + dns_acl_t *inneracl = NULL; + /* + * Convert any named acls we reference now if + * they have not already been converted. + */ + result = convert_named_acl(ce, cctx, lctx, ctx, + mctx, 0, &inneracl); + if (result == ISC_R_SUCCESS) { + if (inneracl->has_negatives) + n++; + else + n += inneracl->length; + dns_acl_detach(&inneracl); + } else + return (result); } } } - return n; + *count = n; + return (ISC_R_SUCCESS); } #ifdef HAVE_GEOIP @@ -594,11 +613,14 @@ cfg_acl_fromconfig2(const cfg_obj_t *caml, const cfg_obj_t *cctx, * elements table. (Note that if nest_level is nonzero, * *everything* goes in the elements table.) */ - int nelem; + isc_uint32_t nelem; - if (nest_level == 0) - nelem = count_acl_elements(caml, cctx, NULL); - else + if (nest_level == 0) { + result = count_acl_elements(caml, cctx, lctx, ctx, + mctx, &nelem, NULL); + if (result != ISC_R_SUCCESS) + return (result); + } else nelem = cfg_list_length(caml, ISC_FALSE); result = dns_acl_create(mctx, nelem, &dacl); @@ -613,6 +635,8 @@ cfg_acl_fromconfig2(const cfg_obj_t *caml, const cfg_obj_t *cctx, const cfg_obj_t *ce = cfg_listelt_value(elt); isc_boolean_t neg = ISC_FALSE; + INSIST(dacl->length <= dacl->alloc); + if (cfg_obj_istuple(ce)) { /* Might be a negated element */ const cfg_obj_t *negated = @@ -667,6 +691,7 @@ cfg_acl_fromconfig2(const cfg_obj_t *caml, const cfg_obj_t *cctx, goto cleanup; if (nest_level > 0) { + INSIST(dacl->length < dacl->alloc); de->type = dns_aclelementtype_nestedacl; de->negative = neg; } else @@ -688,6 +713,7 @@ cfg_acl_fromconfig2(const cfg_obj_t *caml, const cfg_obj_t *cctx, goto cleanup; nested_acl: if (nest_level > 0 || inneracl->has_negatives) { + INSIST(dacl->length < dacl->alloc); de->type = dns_aclelementtype_nestedacl; de->negative = neg; if (de->nestedacl != NULL) @@ -697,14 +723,18 @@ nested_acl: dns_acl_detach(&inneracl); /* Fall through. */ } else { + INSIST(dacl->length + inneracl->length + <= dacl->alloc); dns_acl_merge(dacl, inneracl, ISC_TF(!neg)); de += inneracl->length; /* elements added */ dns_acl_detach(&inneracl); + INSIST(dacl->length <= dacl->alloc); continue; } } else if (cfg_obj_istype(ce, &cfg_type_keyref)) { /* Key name. */ + INSIST(dacl->length < dacl->alloc); de->type = dns_aclelementtype_keyname; de->negative = neg; dns_name_init(&de->keyname, NULL); @@ -716,6 +746,7 @@ nested_acl: } else if (cfg_obj_istuple(ce) && cfg_obj_isvoid(cfg_tuple_get(ce, "negated"))) { + INSIST(dacl->length < dacl->alloc); result = parse_geoip_element(ce, lctx, ctx, de); if (result != ISC_R_SUCCESS) goto cleanup; @@ -733,6 +764,7 @@ nested_acl: goto cleanup; if (nest_level != 0) { + INSIST(dacl->length < dacl->alloc); de->type = dns_aclelementtype_nestedacl; de->negative = neg; } else @@ -754,19 +786,26 @@ nested_acl: dacl->has_negatives = !neg; if (nest_level != 0) { + INSIST(dacl->length < dacl->alloc); de->type = dns_aclelementtype_nestedacl; de->negative = !neg; } else continue; } else if (strcasecmp(name, "localhost") == 0) { + INSIST(dacl->length < dacl->alloc); de->type = dns_aclelementtype_localhost; de->negative = neg; } else if (strcasecmp(name, "localnets") == 0) { + INSIST(dacl->length < dacl->alloc); de->type = dns_aclelementtype_localnets; de->negative = neg; } else { if (inneracl != NULL) dns_acl_detach(&inneracl); + /* + * This call should just find the cached + * of the named acl. + */ result = convert_named_acl(ce, cctx, lctx, ctx, mctx, new_nest_level, &inneracl);