On Thu, 16 Dec 2010 18:01:16 +0100
Kamil Dudka <kdudka_at_redhat.com> wrote:

> On Thursday 16 December 2010 17:17:13 Paul Howarth wrote:
> > Building 7.21.3 with glibc and _FORTIFY_SOURCE=2 results in a crash
> > of the tftp server whilst running the test suite (when it has to
> > process its first error condition):
> >
> > *** buffer overflow detected ***: server/tftpd terminated
> > ======= Backtrace: =========
> > /lib/libc.so.6(__fortify_fail+0x4d)[0x20680d]
> > /lib/libc.so.6(+0xf482a)[0x20482a]
> > /lib/libc.so.6(__strcpy_chk+0x3f)[0x203adf]
> > server/tftpd[0x804c0e1]
> > server/tftpd[0x804d896]
> > /lib/libc.so.6(__libc_start_main+0xe6)[0x126e16]
> > server/tftpd[0x8048fe1]
> >
> > It was easy to identify the source of this problem since there's
> > only one call of strcpy() in tftpd.c. The issue looks very similar
> > to the one described here:
> >
> > https://bugzilla.redhat.com/show_bug.cgi?id=515361
> >
> > However, in this case we *are* writing to the last field...
>
> The structure is embedded into a union and my guess is that it
> confuses the overflow checker.
>
> > Attached patch, based on the suggestion solution in the above
> > ticket, works for me.
>
> The patch looks sane. But the explicit zero termination seems
> redundant to me (in both cases), since the terminating zero is
> already copied as part of the string. Or am I missing something?

Couldn't see why that was there myself either, but I limited the change
to fixing the problem rather than expanding the scope of the change to
clean up the code too (which might be a nice project for someone in the
future...).

> By the way, thanks for the update in rawhide!

No problem - thought you might be busy since the new release had been
out for 24 hours and still not in Rawhide yet ;-)

Paul.
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2010-12-16