From: Theo Buehler <tb@theobuehler.org>
Subject: Re: dig warnings
To: tech@openbsd.org
Date: Fri, 17 May 2024 09:48:55 +0200

On Fri, May 17, 2024 at 09:41:35AM +0200, Florian Obser wrote:
> On 2024-05-17 09:34 +02, Theo Buehler <tb@theobuehler.org> wrote:
> >> would it make sense to do this the other way around? Would a compiler be
> >> able to catch mistakes that way?
> >
> > I was considering this but I could not convince clang to throw a warning.
> > For example, the too short key array in the diff below is fed into
> > isc_sha256_final() in lib/dns/hmac_link.c:hmacsha256_fromdns(), but
> > clang is silent about this.
> >
> > That said, I'm happy to add the array sizes to the function defnitions
> > if there's any benefit to doing so.
> 
> Let's do that then? Maybe a future compiler will be smart enough. I
> can't see a downside in doing it.

Sure, if you prefer that:

Index: lib/isc/sha2.c
===================================================================
RCS file: /cvs/src/usr.bin/dig/lib/isc/sha2.c,v
diff -u -p -r1.4 sha2.c
--- lib/isc/sha2.c	24 Feb 2020 13:49:38 -0000	1.4
+++ lib/isc/sha2.c	17 May 2024 07:46:59 -0000
@@ -87,7 +87,7 @@ isc_sha224_update(isc_sha224_t *context,
 }
 
 void
-isc_sha224_final(uint8_t digest[], isc_sha224_t *context) {
+isc_sha224_final(uint8_t digest[ISC_SHA224_DIGESTLENGTH], isc_sha224_t *context) {
 	/* Sanity check: */
 	REQUIRE(context != (isc_sha224_t *)0);
 	REQUIRE(context->ctx != (EVP_MD_CTX *)0);
@@ -129,7 +129,7 @@ isc_sha256_update(isc_sha256_t *context,
 }
 
 void
-isc_sha256_final(uint8_t digest[], isc_sha256_t *context) {
+isc_sha256_final(uint8_t digest[ISC_SHA256_DIGESTLENGTH], isc_sha256_t *context) {
 	/* Sanity check: */
 	REQUIRE(context != (isc_sha256_t *)0);
 	REQUIRE(context->ctx != (EVP_MD_CTX *)0);
@@ -169,7 +169,7 @@ void isc_sha512_update(isc_sha512_t *con
 				       (const void *) data, len) == 1);
 }
 
-void isc_sha512_final(uint8_t digest[], isc_sha512_t *context) {
+void isc_sha512_final(uint8_t digest[ISC_SHA512_DIGESTLENGTH], isc_sha512_t *context) {
 	/* Sanity check: */
 	REQUIRE(context != (isc_sha512_t *)0);
 	REQUIRE(context->ctx != (EVP_MD_CTX *)0);
@@ -211,7 +211,7 @@ isc_sha384_update(isc_sha384_t *context,
 }
 
 void
-isc_sha384_final(uint8_t digest[], isc_sha384_t *context) {
+isc_sha384_final(uint8_t digest[ISC_SHA384_DIGESTLENGTH], isc_sha384_t *context) {
 	/* Sanity check: */
 	REQUIRE(context != (isc_sha384_t *)0);
 	REQUIRE(context->ctx != (EVP_MD_CTX *)0);