Re: [Xen-devel] [BUG]Buffer Overflow in string library

[Matthew Daley <mattjd@xxxxxxxxx>]

On Sat, Sep 14, 2013 at 9:33 AM, Steve Calandra
<steven.calandra@xxxxxxxxx> wrote:
> There is a potential, though unlikely buffer overflow vulnerability in the
> function strlcpy() in string.c

Which string.c? There are multiple, but I'm guessing xen/common/string.c.

>
> size_t strlcpy(char *dest, const char *src, size_t size)
> {
>     size_t ret = strlen(src);
>     size_t destLen = strLen(dest);

I can't see this (broken?) line in any of Xen's source...?

>
>     if (size) {
>         size_t len = (ret >= size) ? size-1 : ret;
>         memcpy(dest, src, len);
>         dest[len] = '\0';
>     }
>     return ret;
> }
>
> In the event that size is greater than the length of src and dest, dest will
> be overflowed.  This can be fixed with the following:
>
> if (len >= strlen(dest))
>      len = strlen(dest) -1;

Well, 'size' only needs to be bigger than the 'dest' buffer size to
cause a write overflow, but that's moot anyway; strlcpy is a
well-known function provided by many C standard libraries, and it
provides no claims as to the safety of calling it with a 'size' bigger
than the 'dest' buffer size.

See, for example,
http://www.openbsd.org/cgi-bin/man.cgi?query=strlcpy&sektion=3 . The
version in xen/common/string.c is for when it's not provided by the
system C library (ie. with glibc), that's why it's wrapped in '#ifndef
__HAVE_ARCH_STRLCPY'.

Also, using strlen(dest) wouldn't work as there is no requirement for
'dest' to already be a valid string, only a valid pointer to a
writeable buffer of at least size 'size'. Perhaps you've confused
strlcpy with strlcat?

>
> I tried fixing it myself, but I was having problems pushing the change to
> the repo.

Only committers can push (and things go through osstest first anyway),
assuming you're talking about a repo on xenbits.xen.org.

- Matthew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel