[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Crash of libvirtd by unprivileged user in virConnectListAllInterfaces



On 07/02/2013 10:14 PM, Eric Blake wrote:
From: "Daniel P. Berrange" <berrange redhat com>

On Thu, Jun 27, 2013 at 03:56:42PM +0100, Daniel P. Berrange wrote:
Hi Security Team,

I've discovered a way for an unprivileged user with a readonly connection
to libvirtd, to crash the daemon.
Ok, the final patch for this is issue will be the simpler variant that
Eric suggested

The embargo can be considered to be lifted on Monday July 1st, at
0900 UTC

The following is the GIT change that DV or myself will apply to libvirt
GIT master immediately before the 1.1.0 release:

>From 177b4165c531a4b3ba7f6ab6aa41dca9ceb0b8cf Mon Sep 17 00:00:00 2001
From: "Daniel P. Berrange" <berrange redhat com>
Date: Fri, 28 Jun 2013 10:48:37 +0100
Subject: [PATCH] CVE-2013-2218: Fix crash listing network interfaces with
  filters

The virConnectListAllInterfaces method has a double-free of the
'struct netcf_if' object when any of the filtering flags cause
an interface to be skipped over. For example when running the
command 'virsh iface-list --inactive'

This is a regression introduced in release 1.0.6 by

   commit 7ac2c4fe624f30f2c8270116513fa2ddab07631f
   Author: Guannan Ren <gren redhat com>
   Date:   Tue May 21 21:29:38 2013 +0800

     interface: list all interfaces with flags == 0

Signed-off-by: Daniel P. Berrange <berrange redhat com>
---

Posting as a courtesy FYI for anyone reading this list but who does
not have access to the security list and doesn't want to crawl
through git.  This commit has been included in 1.1.0 and has been
applied to all affected stable branches (just v1.0.6-maint).

The rule in determining that a CVE was necessary is the
"escalation of privilege" test - any time a read-only client can
cause a denial-of-service against a more-privileged read-write
client (by crashing libvirtd), there is an escalation.

  src/interface/interface_backend_netcf.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c
index a995816..9aa673d 100644
--- a/src/interface/interface_backend_netcf.c
+++ b/src/interface/interface_backend_netcf.c
@@ -412,6 +412,7 @@ netcfConnectListAllInterfaces(virConnectPtr conn,
                (MATCH(VIR_CONNECT_LIST_INTERFACES_INACTIVE) &&
                 (status & NETCF_IFACE_INACTIVE)))) {
              ncf_if_free(iface);
+            iface = NULL;
              continue;
          }


Thanks for posting here. I have to say sorry about this bug I caused.
This problem is caused by double-free 'iface' variable as Daniel pointed out
when code path goes into cleanup label, there is a second ncf_if_free()
for the variable if we didn't initialize it to NULL after the first ncf_if_free().

Guannan


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]