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

In the following recent change:

  commit db72866310
  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>
This commit is contained in:
Daniel P. Berrangé 2020-03-05 15:42:53 +00:00
parent c04e25b66c
commit 415e022118
4 changed files with 67 additions and 2 deletions

View File

@ -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:

View File

@ -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 \

57
tests/virsh-auth Executable file
View File

@ -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)

5
tests/virsh-auth.xml Normal file
View File

@ -0,0 +1,5 @@
<node>
<auth>
<user>astrochicken</user>
</auth>
</node>