[libvirt PATCH v2] src: fix mixup of stack and heap allocated data in auth callback

[Daniel P. Berrangé]

In the following recent change:

  commit db72866310d1e520efa8ed2d4589bdb5e76a1c95
  Author: Daniel P. Berrangé <berrange at 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.

Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
---

Changed in v2:

 - Keep use of fgets for mingw portability, but strdup the
   static buffer

 src/libvirt.c        |  5 ++--
 tests/Makefile.am    |  2 ++
 tests/virsh-auth     | 57 ++++++++++++++++++++++++++++++++++++++++++++
 tests/virsh-auth.xml |  5 ++++
 4 files changed, 67 insertions(+), 2 deletions(-)
 create mode 100755 tests/virsh-auth
 create mode 100644 tests/virsh-auth.xml

diff --git a/src/libvirt.c b/src/libvirt.c
index a30eaa7590..76bf1fa677 100644
--- 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 83326dbaa9..3b5abcc12b 100644
--- 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
index 0000000000..d548694190
--- /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
index 0000000000..a045a0746e
--- /dev/null
+++ b/tests/virsh-auth.xml
@@ -0,0 +1,5 @@
+<node>
+  <auth>
+    <user>astrochicken</user>
+  </auth>
+</node>
-- 
2.24.1