src: fix mixup of stack and heap allocated data in auth callback

In the following recent change:

  commit db72866310d1e520efa8ed2d4589bdb5e76a1c95
  Author: Daniel P. Berrangé <berrange@redhat.com>
  Date:   Tue Jan 14 10:40:52 2020 +0000

    util: add API for reading password from the console

the fact that "bufptr" pointer may point to either heap or stack
allocated data was overlooked. As a result, when the strdup was
removed, we ended up returning a pointer to the local stack to
the caller. When the caller referenced this stack pointer they
got out garbage which fairly quickly resulted in a crash.

We need to copy the stack buffer into heap memory in the username
case.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/src/libvirt.c b/src/libvirt.c
index a30eaa7590427b38f3886680b00448322361d75a..76bf1fa6772dea12a645e895bcb6c59a64a5f727 100644 (file)
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -111,7 +111,7 @@ virConnectAuthCallbackDefault(virConnectCredentialPtr cred,
 
     for (i = 0; i < ncred; i++) {
         char buf[1024];
-        char *bufptr = buf;
+        char *bufptr = NULL;
         size_t len;
 
         switch (cred[i].type) {
@@ -138,14 +138,15 @@ virConnectAuthCallbackDefault(virConnectCredentialPtr cred,
 
             if (!fgets(buf, sizeof(buf), stdin)) {
                 if (feof(stdin)) { /* Treat EOF as "" */
-                    buf[0] = '\0';
                     break;
                 }
                 return -1;
             }
+
             len = strlen(buf);
             if (len != 0 && buf[len-1] == '\n')
                 buf[len-1] = '\0';
+            bufptr = g_strdup(buf);
             break;
 
         case VIR_CRED_PASSPHRASE:

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 83326dbaa9eb0f9483984c1562d79d0493b2e3fa..3b5abcc12bfd009c4538538e6c685fa4ca4553e4 100644 (file)
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -164,6 +164,7 @@ EXTRA_DIST = \
        xlconfigdata \
        xmconfigdata \
        xml2vmxdata \
+       virsh-auth.xml \
        virstorageutildata \
        virfilecachedata \
        virresctrldata \
@@ -406,6 +407,7 @@ test_scripts =
 libvirtd_test_scripts = \
        libvirtd-fail \
        libvirtd-pool \
+       virsh-auth \
        virsh-cpuset \
        virsh-define-dev-segfault \
        virsh-int-overflow \

diff --git a/tests/virsh-auth b/tests/virsh-auth
new file mode 100755 (executable)
index 0000000..d548694
--- /dev/null
+++ b/tests/virsh-auth
@@ -0,0 +1,57 @@
+#!/usr/bin/env python3
+# run virsh to validate interactive auth
+
+# Copyright (C) 2020 Red Hat, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see
+# <http://www.gnu.org/licenses/>.
+
+import os
+import os.path
+import sys
+import subprocess
+
+builddir = os.getenv("abs_top_builddir")
+if builddir is None:
+   builddir = os.path.join(os.getcwd(), "..")
+
+srcdir = os.getenv("abs_top_srcdir")
+if srcdir is None:
+   srcdir = os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))
+
+uri = "test://" + os.path.join(srcdir, "tests", "virsh-auth.xml")
+
+virsh = os.path.join(builddir, "tools", "virsh")
+
+proc = subprocess.Popen([virsh, "-c", uri, "uri"],
+                        universal_newlines=True,
+                        start_new_session=True,
+                        stdin=subprocess.PIPE,
+                        stdout=subprocess.PIPE,
+                        stderr=subprocess.PIPE)
+out, err = proc.communicate("astrochicken")
+
+if proc.returncode != 0:
+   print("virsh failed with code %d" % proc.returncode, file=sys.stderr)
+   if out != "":
+      print("stdout=%s" % out)
+   if err != "":
+      print("stderr=%s" % err)
+   sys.exit(1)
+
+if uri not in out:
+   print("Expected '%s' in '%s'" % (uri, out), file=sys.stderr)
+   sys.exit(1)
+
+sys.exit(0)

diff --git a/tests/virsh-auth.xml b/tests/virsh-auth.xml
new file mode 100644 (file)
index 0000000..a045a07
--- /dev/null
+++ b/tests/virsh-auth.xml
@@ -0,0 +1,5 @@
+<node>
+  <auth>
+    <user>astrochicken</user>
+  </auth>
+</node>