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. Signed-off-by: Daniel P. Berrangé <berrange@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