[ntp:hackers] MD5auth_setkey bug in copying message digest

Dave Hart davehart at gmail.com
Mon Jul 6 14:03:07 UTC 2009


On Mon, Jul 6, 2009 at 1:07 AM, Victor Jesus Angus wrote:
>
> Hello,
>
> I'm implementing a small autokey client that specifically works using IFF
> and found a bug in the reference implementation
> libntp/authkeys.c:MD5auth_setkey. When copying the key to sk->k.MD5_key,
> it uses strncpy which causes the destination to be truncated when a NULL is
> found which causes to generate a wrong MAC. My proposed fix is to use
> memcpy.

Hi Victor,

Please file a bug report at http://bugs.ntp.org/ and attach a proposed
patch using the website's Add Attachment link.  Do not be surprised if
your browser warns you about the https certificate, it is issued by
cacert.org, which many browsers do not trust by default.

> -                       strncpy((char *)sk->k.MD5_key, (const char *)key,
> -                           sizeof(sk->k.MD5_key));
> +                       memcpy(sk->k.MD5_key, key, len);
>                        if ((sk->keylen = len) > sizeof(sk->k.MD5_key))
>                            sk->keylen = sizeof(sk->k.MD5_key);

Before submitting your patch, I suggest you change it to more clearly
protect against buffer overrun by moving the memcpy() after sk->keylen
is set and using sk->keylen as the number of bytes to copy for
memcpy(), in both snippets.

Cheers,
Dave Hart


More information about the hackers mailing list