[libvirt] [PATCH] proxy: Fix use of uninitalized memory
On short read, members of packet header are checked before actually read. If uninitialized values can pass the test, they can be set to arbitrary values while reading remaining portion of a packet. Buffer overflow is possible. libvirt_proxy is suid-root. diff -urp libvirt-0.5.1/proxy/libvirt_proxy.c libvirt-dev/proxy/libvirt_proxy.c --- libvirt-0.5.1/proxy/libvirt_proxy.c 2008-11-20 08:58:43.000000000 +0100 +++ libvirt-dev/proxy/libvirt_proxy.c 2009-01-25 12:51:33.000000000 +0100 @@ -385,7 +385,8 @@ retry: fprintf(stderr, "read %d bytes from client %d on socket %d\n", ret, nr, pollInfos[nr].fd); - if ((req->version != PROXY_PROTO_VERSION) || + if ((ret != sizeof(virProxyPacket)) || + (req->version != PROXY_PROTO_VERSION) || (req->len < sizeof(virProxyPacket)) || (req->len > sizeof(virProxyFullPacket))) goto comm_error;
"Rasputin" <rasputin@email.ru> wrote:
On short read, members of packet header are checked before actually read. If uninitialized values can pass the test, they can be set to arbitrary values while reading remaining portion of a packet.
Buffer overflow is possible. libvirt_proxy is suid-root.
diff -urp libvirt-0.5.1/proxy/libvirt_proxy.c libvirt-dev/proxy/libvirt_proxy.c --- libvirt-0.5.1/proxy/libvirt_proxy.c 2008-11-20 08:58:43.000000000 +0100 +++ libvirt-dev/proxy/libvirt_proxy.c 2009-01-25 12:51:33.000000000 +0100 @@ -385,7 +385,8 @@ retry: fprintf(stderr, "read %d bytes from client %d on socket %d\n", ret, nr, pollInfos[nr].fd);
- if ((req->version != PROXY_PROTO_VERSION) || + if ((ret != sizeof(virProxyPacket)) || + (req->version != PROXY_PROTO_VERSION) || (req->len < sizeof(virProxyPacket)) || (req->len > sizeof(virProxyFullPacket))) goto comm_error;
This looks like a good patch. Thanks!
"Rasputin" <rasputin@email.ru> wrote:
On short read, members of packet header are checked before actually read. If uninitialized values can pass the test, they can be set to arbitrary values while reading remaining portion of a packet.
Buffer overflow is possible. libvirt_proxy is suid-root.
diff -urp libvirt-0.5.1/proxy/libvirt_proxy.c libvirt-dev/proxy/libvirt_proxy.c --- libvirt-0.5.1/proxy/libvirt_proxy.c 2008-11-20 08:58:43.000000000 +0100 +++ libvirt-dev/proxy/libvirt_proxy.c 2009-01-25 12:51:33.000000000 +0100 @@ -385,7 +385,8 @@ retry: fprintf(stderr, "read %d bytes from client %d on socket %d\n", ret, nr, pollInfos[nr].fd);
- if ((req->version != PROXY_PROTO_VERSION) || + if ((ret != sizeof(virProxyPacket)) || + (req->version != PROXY_PROTO_VERSION) || (req->len < sizeof(virProxyPacket)) || (req->len > sizeof(virProxyFullPacket))) goto comm_error;
Thanks again. Here's what I expect to commit. If you would like different attribution, let me know.
From 0abc169e868d7b7b5d1edd75aec0e021b1e67c53 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Wed, 28 Jan 2009 14:27:11 +0100 Subject: [PATCH] libvirt_proxy: avoid potential buffer overflow
* proxy/libvirt_proxy.c (proxyReadClientSocket): Ensure that we've read an entire virProxyPacket before dereferencing "req". Analysis and patch by "Rasputin" <rasputin@email.ru>. Details in <http://thread.gmane.org/gmane.comp.emulators.libvirt/11459>. --- proxy/libvirt_proxy.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/proxy/libvirt_proxy.c b/proxy/libvirt_proxy.c index f3d9ede..863dc32 100644 --- a/proxy/libvirt_proxy.c +++ b/proxy/libvirt_proxy.c @@ -2,7 +2,7 @@ * proxy_svr.c: root suid proxy server for Xen access to APIs with no * side effects from unauthenticated clients. * - * Copyright (C) 2006, 2007, 2008 Red Hat, Inc. + * Copyright (C) 2006, 2007, 2008, 2009 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -382,7 +382,8 @@ retry: fprintf(stderr, "read %d bytes from client %d on socket %d\n", ret, nr, pollInfos[nr].fd); - if ((req->version != PROXY_PROTO_VERSION) || + if ((ret != sizeof(virProxyPacket)) || + (req->version != PROXY_PROTO_VERSION) || (req->len < sizeof(virProxyPacket)) || (req->len > sizeof(virProxyFullPacket))) goto comm_error; -- 1.6.1.1.374.g0d9d7
participants (2)
-
Jim Meyering -
Rasputin