From nobody Mon Apr 29 21:10:01 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 207.211.31.120 as permitted sender) client-ip=207.211.31.120; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-1.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 207.211.31.120 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1583427341; cv=none; d=zohomail.com; s=zohoarc; b=O+H46SBQlFmwEdFi4Bz+OxiPSy8zjc5SZgVOK7XZDXwQnR6G3KeazdR2nNAjR8hzUpJX8QJGxFJaUlglvMWZQ8UfvgHQyXjRenf6UVAu5gbJvacu3dOtWorK6E7Hul052+9yfP36VmRQdxqgPRGtRcTQg04bDMw/bxT2u/ZS5AA= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1583427341; h=Content-Type:Content-Transfer-Encoding:Date:From:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:To; bh=o4puZqblZQGnCCSE6w6pLqSrkEduL8iGn8GWlUhoPas=; b=Ncc967oFfwpG7UEtAuntfnRIz0tfRXTIQigG25TpZpDIFznq78wDiPyGDnZe63Ar6VJ172GwGpkBYV2hfeCyTMGIJUNuSLBf5tTZYdWATOtAoH/C7gT0KHJwhcWUBMUTMJoLOJEWbBWmT9GVwDUxDEQDiZPza2ZH7Pa7APmC3LI= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 207.211.31.120 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) header.from= Return-Path: Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.120]) by mx.zohomail.com with SMTPS id 1583427341203243.06309725508402; Thu, 5 Mar 2020 08:55:41 -0800 (PST) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-1-BWJfwVmUPh-ZMCZKmhXBuQ-1; Thu, 05 Mar 2020 11:55:37 -0500 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 480BF8017CC; Thu, 5 Mar 2020 16:55:32 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 1FA8139A; Thu, 5 Mar 2020 16:55:32 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id C4C1886A05; Thu, 5 Mar 2020 16:55:31 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 025GsKHf025352 for ; Thu, 5 Mar 2020 11:54:20 -0500 Received: by smtp.corp.redhat.com (Postfix) id 28148271BA; Thu, 5 Mar 2020 16:54:20 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-112-77.ams2.redhat.com [10.36.112.77]) by smtp.corp.redhat.com (Postfix) with ESMTP id 02CF52719A; Thu, 5 Mar 2020 16:54:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1583427340; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=o4puZqblZQGnCCSE6w6pLqSrkEduL8iGn8GWlUhoPas=; b=OXJAI8RqEKznrlVOQjmFYxc9l9LLVTXae8FRuEJTqclz+8bgF+yBqKdua02UpudpftrT0s r5NJbTOPeWgcuGvGv6GuyuIHNE6uG6RqqxKors6z8EyHNyJ4k8ZQZpGTjAouJd+wbS9jD9 In/P3iCtD7Mkhgadpvxn4rwRtTpHsY4= X-MC-Unique: BWJfwVmUPh-ZMCZKmhXBuQ-1 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= To: libvir-list@redhat.com Subject: [libvirt PATCH] src: fix mixup of stack and heap allocated data in auth callback Date: Thu, 5 Mar 2020 16:54:10 +0000 Message-Id: <20200305165410.2741543-1-berrange@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-loop: libvir-list@redhat.com X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) Content-Type: text/plain; charset="utf-8" In the following recent change: commit db72866310d1e520efa8ed2d4589bdb5e76a1c95 Author: Daniel P. Berrang=C3=A9 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. Switching from fgets() to getline() lets to avoid the need for the stack allocated buffer entirely, which makes both cases of the switch consistent. Test case addition is inspired by the libguestfs test which caught this bug. Signed-off-by: Daniel P. Berrang=C3=A9 --- src/libvirt.c | 30 +++++++++++------------ tests/Makefile.am | 2 ++ tests/virsh-auth | 57 ++++++++++++++++++++++++++++++++++++++++++++ tests/virsh-auth.xml | 5 ++++ 4 files changed, 79 insertions(+), 15 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..cc9c2eb5a2 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -110,9 +110,8 @@ virConnectAuthCallbackDefault(virConnectCredentialPtr c= red, size_t i; =20 for (i =3D 0; i < ncred; i++) { - char buf[1024]; - char *bufptr =3D buf; - size_t len; + char *buf =3D NULL; + size_t len =3D 0; =20 switch (cred[i].type) { case VIR_CRED_EXTERNAL: { @@ -136,16 +135,17 @@ virConnectAuthCallbackDefault(virConnectCredentialPtr= cred, if (fflush(stdout) !=3D 0) return -1; =20 - if (!fgets(buf, sizeof(buf), stdin)) { - if (feof(stdin)) { /* Treat EOF as "" */ - buf[0] =3D '\0'; - break; + if (getline(&buf, &len, stdin) < 0) { + if (!feof(stdin)) { + return -1; } - return -1; + /* Use creddefault on EOF */ + buf =3D NULL; + } else { + len =3D strlen(buf); + if (len !=3D 0 && buf[len-1] =3D=3D '\n') + buf[len-1] =3D '\0'; } - len =3D strlen(buf); - if (len !=3D 0 && buf[len-1] =3D=3D '\n') - buf[len-1] =3D '\0'; break; =20 case VIR_CRED_PASSPHRASE: @@ -155,9 +155,9 @@ virConnectAuthCallbackDefault(virConnectCredentialPtr c= red, if (fflush(stdout) !=3D 0) return -1; =20 - bufptr =3D virGetPassword(); - if (STREQ(bufptr, "")) - VIR_FREE(bufptr); + buf =3D virGetPassword(); + if (STREQ(buf, "")) + VIR_FREE(buf); break; =20 default: @@ -165,7 +165,7 @@ virConnectAuthCallbackDefault(virConnectCredentialPtr c= red, } =20 if (cred[i].type !=3D VIR_CRED_EXTERNAL) { - cred[i].result =3D bufptr ? bufptr : g_strdup(cred[i].defresul= t ? cred[i].defresult : ""); + cred[i].result =3D buf ? buf : g_strdup(cred[i].defresult ? cr= ed[i].defresult : ""); cred[i].resultlen =3D strlen(cred[i].result); } } 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 =3D \ xlconfigdata \ xmconfigdata \ xml2vmxdata \ + virsh-auth.xml \ virstorageutildata \ virfilecachedata \ virresctrldata \ @@ -406,6 +407,7 @@ test_scripts =3D libvirtd_test_scripts =3D \ 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 +# . + +import os +import os.path +import sys +import subprocess + +builddir =3D os.getenv("abs_top_builddir") +if builddir is None: + builddir =3D os.path.join(os.getcwd(), "..") + +srcdir =3D os.getenv("abs_top_srcdir") +if srcdir is None: + srcdir =3D os.path.abspath(os.path.join(os.path.dirname(__file__), ".."= )) + +uri =3D "test://" + os.path.join(srcdir, "tests", "virsh-auth.xml") + +virsh =3D os.path.join(builddir, "tools", "virsh") + +proc =3D subprocess.Popen([virsh, "-c", uri, "uri"], + universal_newlines=3DTrue, + start_new_session=3DTrue, + stdin=3Dsubprocess.PIPE, + stdout=3Dsubprocess.PIPE, + stderr=3Dsubprocess.PIPE) +out, err =3D proc.communicate("astrochicken") + +if proc.returncode !=3D 0: + print("virsh failed with code %d" % proc.returncode, file=3Dsys.stderr) + if out !=3D "": + print("stdout=3D%s" % out) + if err !=3D "": + print("stderr=3D%s" % err) + sys.exit(1) + +if uri not in out: + print("Expected '%s' in '%s'" % (uri, out), file=3Dsys.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 @@ + + + astrochicken + + --=20 2.24.1