From 78066157145b6a75f58ff843ac32ffabe62b2143 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 1 Nov 2021 11:40:26 +1100 Subject: [PATCH] Address bugs in opensslrsa_tofile 1) if 'key->external' is set we just need to call dst__privstruct_writefile 2) the cleanup of 'bufs' was incorrect as 'i' doesn't reflect the the current index into 'bufs'. Use a simple for loop. This review was triggered by Coverity reporting a buffer overrun on 'bufs'. --- lib/dns/opensslrsa_link.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/dns/opensslrsa_link.c b/lib/dns/opensslrsa_link.c index 7ac50e765b..2edf0c98c3 100644 --- a/lib/dns/opensslrsa_link.c +++ b/lib/dns/opensslrsa_link.c @@ -811,7 +811,7 @@ opensslrsa_tofile(const dst_key_t *key, const char *directory) { if (key->external) { priv.nelements = 0; - DST_RET(dst__privstruct_writefile(key, &priv, directory)); + return (dst__privstruct_writefile(key, &priv, directory)); } pkey = key->keydata.pkey; @@ -855,6 +855,7 @@ opensslrsa_tofile(const dst_key_t *key, const char *directory) { if (d != NULL) { priv.elements[i].tag = TAG_RSA_PRIVATEEXPONENT; priv.elements[i].length = BN_num_bytes(d); + INSIST(i < ARRAY_SIZE(bufs)); bufs[i] = isc_mem_get(key->mctx, priv.elements[i].length); BN_bn2bin(d, bufs[i]); priv.elements[i].data = bufs[i]; @@ -864,6 +865,7 @@ opensslrsa_tofile(const dst_key_t *key, const char *directory) { if (p != NULL) { priv.elements[i].tag = TAG_RSA_PRIME1; priv.elements[i].length = BN_num_bytes(p); + INSIST(i < ARRAY_SIZE(bufs)); bufs[i] = isc_mem_get(key->mctx, priv.elements[i].length); BN_bn2bin(p, bufs[i]); priv.elements[i].data = bufs[i]; @@ -873,6 +875,7 @@ opensslrsa_tofile(const dst_key_t *key, const char *directory) { if (q != NULL) { priv.elements[i].tag = TAG_RSA_PRIME2; priv.elements[i].length = BN_num_bytes(q); + INSIST(i < ARRAY_SIZE(bufs)); bufs[i] = isc_mem_get(key->mctx, priv.elements[i].length); BN_bn2bin(q, bufs[i]); priv.elements[i].data = bufs[i]; @@ -882,6 +885,7 @@ opensslrsa_tofile(const dst_key_t *key, const char *directory) { if (dmp1 != NULL) { priv.elements[i].tag = TAG_RSA_EXPONENT1; priv.elements[i].length = BN_num_bytes(dmp1); + INSIST(i < ARRAY_SIZE(bufs)); bufs[i] = isc_mem_get(key->mctx, priv.elements[i].length); BN_bn2bin(dmp1, bufs[i]); priv.elements[i].data = bufs[i]; @@ -891,6 +895,7 @@ opensslrsa_tofile(const dst_key_t *key, const char *directory) { if (dmq1 != NULL) { priv.elements[i].tag = TAG_RSA_EXPONENT2; priv.elements[i].length = BN_num_bytes(dmq1); + INSIST(i < ARRAY_SIZE(bufs)); bufs[i] = isc_mem_get(key->mctx, priv.elements[i].length); BN_bn2bin(dmq1, bufs[i]); priv.elements[i].data = bufs[i]; @@ -900,6 +905,7 @@ opensslrsa_tofile(const dst_key_t *key, const char *directory) { if (iqmp != NULL) { priv.elements[i].tag = TAG_RSA_COEFFICIENT; priv.elements[i].length = BN_num_bytes(iqmp); + INSIST(i < ARRAY_SIZE(bufs)); bufs[i] = isc_mem_get(key->mctx, priv.elements[i].length); BN_bn2bin(iqmp, bufs[i]); priv.elements[i].data = bufs[i]; @@ -926,7 +932,7 @@ opensslrsa_tofile(const dst_key_t *key, const char *directory) { ret = dst__privstruct_writefile(key, &priv, directory); err: - while (i--) { + for (i = 0; i < ARRAY_SIZE(bufs); i++) { if (bufs[i] != NULL) { isc_mem_put(key->mctx, bufs[i], priv.elements[i].length);