From owner-cvs-src@FreeBSD.ORG Mon Apr 4 15:51:27 2005 Return-Path: Delivered-To: cvs-src@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 4CF9116A4CE; Mon, 4 Apr 2005 15:51:27 +0000 (GMT) Received: from gw.celabo.org (gw.celabo.org [208.42.49.153]) by mx1.FreeBSD.org (Postfix) with ESMTP id DA79643D4C; Mon, 4 Apr 2005 15:51:26 +0000 (GMT) (envelope-from nectar@FreeBSD.org) Received: from gw.celabo.org (localhost [127.0.0.1]) by internal.gw.celabo.org (Postfix) with ESMTP id 9C7BF3E2E6F; Mon, 4 Apr 2005 10:51:13 -0500 (CDT) Received: from lum.celabo.org (lum.celabo.org [10.0.1.107]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "lum.celabo.org", Issuer "celabo.org CA" (verified OK)) by gw.celabo.org (Postfix) with ESMTP id 7267B3E2C2A; Mon, 4 Apr 2005 10:51:13 -0500 (CDT) Received: from [127.0.0.1] (localhost [127.0.0.1]) by lum.celabo.org (Postfix) with ESMTP id CAEB76E7929; Mon, 4 Apr 2005 10:51:10 -0500 (CDT) In-Reply-To: <86oecur8ie.fsf@xps.des.no> References: <200503271359.j2RDxiF9050487@repoman.freebsd.org> <86oecur8ie.fsf@xps.des.no> Mime-Version: 1.0 (Apple Message framework v619.2) Content-Type: text/plain; charset=ISO-8859-1; format=flowed Message-Id: <8e5139a9831f2423984641ee24e2a2a3@FreeBSD.org> Content-Transfer-Encoding: quoted-printable From: Jacques Vidrine Date: Mon, 4 Apr 2005 10:51:09 -0500 To: des@des.no (=?ISO-8859-1?Q?Dag-Erling_Sm=F8rgrav?=) X-Mailer: Apple Mail (2.619.2) X-Spam-Checker-Version: SpamAssassin 3.0.2 (2004-11-16) on hellblazer.celabo.org X-Spam-Level: X-Spam-Status: No, score=-5.9 required=5.0 tests=ALL_TRUSTED,BAYES_00 autolearn=ham version=3.0.2 cc: cvs-src@FreeBSD.org cc: src-committers@FreeBSD.org cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/libexec/rexecd rexecd.c X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 04 Apr 2005 15:51:27 -0000 On Apr 4, 2005, at 8:12 AM, Dag-Erling Sm=F8rgrav wrote: > Jacques Vidrine writes: >> A separate bug was introduced at the same time. The PAM library >> functions are called between the invocation of getpwnam(3) and the=20= >> use >> of the returned static object. Since many PAM library functions >> result in additional getpwnam(3) calls, the contents of the = returned >> static object could be changed from under rexecd. With this = commit, >> getpwnam_r(3) is used instead. > > This is incorrect, because PAM may change the login name, so the > struct passwd you got before calling PAM might not be the one you > actually need. The simplest fix is to revert this patch and instead > add > > pam_get_item(pamh, PAM_USER, &user); > pwd =3D getpwnam(user); > > after the PAM transaction. Thanks, I understand what you are saying. This bug existed prior to my=20= commit, but it due to the bug that that commit fixes, it may have been=20= masked in many situations. The fix you suggest also will not work, since other interfaces are=20 invoked between getpwnam() and the final usage of the result structure.=20= To be clear, we started with: pwd =3D getpwnam() if (pwd->pw_uid =3D 0) <--- oops pam_authenticate etc <--- underlying static struct passwd changes setlogin(pwd->pw_name) and other uses of static struct passwd pam_setcred etc <--- possibly more changes to static data setuid(pwd->pw_uid) and other uses of static struct passwd In -CURRENT we now have pwd =3D getpwnam_r() if (pwd->pw_uid =3D=3D 0) pam_authenticate etc <--- Now OK, we have our own struct passwd setlogin(pwd->pw_name) and other uses of local struct passwd pam_setcred etc <--- Now OK, we have our own struct passwd setuid(pwd->pw_uid) and other uses of local struct passwd You bring up that we need to lookup the user again after=20 pam_authenticate. So we will finally need: pwd =3D getpwnam_r() if (pwd->pw_uid =3D=3D 0) pam_authenticate etc pwd =3D getpwnam_r(PAM_USER) <--- user may have changed setlogin(pwd->pw_name) pam_setcred etc setuid(pwd->pw_uid) Thanks for the pointer! By the way, have you checked other PAM-using=20 applications to make sure they do not make the same kinds of mistakes? Cheers, --=20 Jacques A Vidrine / NTT/Verio nectar@celabo.org / jvidrine@verio.net / nectar@freebsd.org