Re: Alignment issue with sha1 code from gnulib

[Jim Meyering]

Jim Meyering <address@hidden> wrote:
> David Shaw <address@hidden> wrote:
>> Peter Palfrader reported a bug against the sha1 code in paperkey, but
>> that code actually comes from gnulib, so I'm referring it to you.
>>
>> The issue comes up (as noted in the comment) if resbuf is not 32-bit
>> aligned.  Rather than requiring all programs that use the gnulib sha1
>> code to align their result buffer, Peter's patch seems to make more
>> sense in that it "just works" on any platform.
>>
>> David
>>
>> ----- Forwarded message from Peter Palfrader <address@hidden> -----
>>
>> This seems to fix the issue:
>>
>> -sha1_read_ctx (const struct sha1_ctx *ctx, void *resbuf)
>> +sha1_read_ctx (const struct sha1_ctx *ctx, char *resbuf)
>>  {
>> -  ((uint32_t *) resbuf)[0] = SWAP (ctx->A);
>> -  ((uint32_t *) resbuf)[1] = SWAP (ctx->B);
>> -  ((uint32_t *) resbuf)[2] = SWAP (ctx->C);
>> -  ((uint32_t *) resbuf)[3] = SWAP (ctx->D);
>> -  ((uint32_t *) resbuf)[4] = SWAP (ctx->E);
>> +  set_uint32(&resbuf[0*4], SWAP (ctx->A));
>> +  set_uint32(&resbuf[1*4], SWAP (ctx->B));
>> +  set_uint32(&resbuf[2*4], SWAP (ctx->C));
>> +  set_uint32(&resbuf[3*4], SWAP (ctx->D));
>> +  set_uint32(&resbuf[4*4], SWAP (ctx->E));
>
> Thanks for the suggestion.  It looks like a good one.
> However, I don't want to change the type of the resbuf parameter.
> Here's the change I'm considering:
>
> diff --git a/lib/sha1.c b/lib/sha1.c
> index 035f898..d9e99d8 100644
> --- a/lib/sha1.c
> +++ b/lib/sha1.c
> @@ -67,28 +67,32 @@ sha1_init_ctx (struct sha1_ctx *ctx)
>    ctx->buflen = 0;
>  }
>
> -/* Put result from CTX in first 20 bytes following RESBUF.  The result
> -   must be in little endian byte order.
> +/* Copy the 4 byte value from v into the memory location pointed to by *cp,
> +   If your architecture allows unaligned access this is equivalent to
> +   * (uint32_t *) cp = v  */
> +void
> +set_uint32 (char *cp, uint32_t v)
> +{
> +  memcpy (cp, &v, 4);
> +}

Oops.
While applying a nearly identical patch to md5.c,
I noticed a minor error: that new function must have static scope.
I'll fix that in sha1.c shortly.