From: Florian Obser <florian@openbsd.org>
Subject: Re: dig warnings
To: Theo Buehler <tb@theobuehler.org>
Cc: tech@openbsd.org
Date: Fri, 17 May 2024 09:41:35 +0200

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.

>
> diff --git a/usr.bin/dig/lib/dns/hmac_link.c b/usr.bin/dig/lib/dns/hmac_link.c
> index 56c886b9e2f..423a910f4b1 100644
> --- a/usr.bin/dig/lib/dns/hmac_link.c
> +++ b/usr.bin/dig/lib/dns/hmac_link.c
> @@ -338,7 +338,7 @@ dst__hmacsha224_init(dst_func_t **funcp) {
>  static isc_result_t hmacsha256_fromdns(dst_key_t *key, isc_buffer_t *data);
>  
>  struct dst_hmacsha256_key {
> -	unsigned char key[ISC_SHA256_BLOCK_LENGTH];
> +	unsigned char key[ISC_SHA224_DIGESTLENGTH];
>  };
>  
>  static isc_result_t
> diff --git a/usr.bin/dig/lib/isc/sha2.c b/usr.bin/dig/lib/isc/sha2.c
> index e1c729d95d5..69915540820 100644
> --- a/usr.bin/dig/lib/isc/sha2.c
> +++ b/usr.bin/dig/lib/isc/sha2.c
> @@ -129,7 +129,7 @@ isc_sha256_update(isc_sha256_t *context, const uint8_t *data, size_t len) {
>  }
>  
>  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);
>

-- 
In my defence, I have been left unsupervised.