Re: [libvirt] [PATCH] Fix latent buffer overflow in qemudOpenMonitorUnix.

On Fri, Jul 31, 2009 at 03:20:09PM +0200, Chris Lalancette wrote: > Fix a possible latent bug in qemudOpenMonitorUnix(). If the pathname > to the monitor is very long (i.e. >= UNIX_MAX_PATH), then strncpy will > *not* place a final \0 on the string (see "man strncpy"). > NULL terminate the buffer to ensure we don't run off the end. > > Signed-off-by: Chris Lalancette <clalance(a)redhat.com&gt; > --- > src/qemu_driver.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/src/qemu_driver.c b/src/qemu_driver.c > index 9fcc07a..4f173b7 100644 > --- a/src/qemu_driver.c > +++ b/src/qemu_driver.c > @@ -910,6 +910,7 @@ qemudOpenMonitorUnix(virConnectPtr conn, > memset(&addr, 0, sizeof(addr)); > addr.sun_family = AF_UNIX; > strncpy(addr.sun_path, monitor, sizeof(addr.sun_path)); > + NUL_TERMINATE(addr.sun_path); > > do { > ret = connect(monfd, (struct sockaddr *) &addr, sizeof(addr)); ACK, but this mistake is made in pretty much every use of unix domain sockets :-( Could you fix the others here too. Clearly strncpy() is just unfit for purpose, and we should plan to banish it in favour of a virStrncpy impl that guarnetees to add the trailing \0.