Date: Mon, 4 Apr 2005 10:51:09 -0500 From: Jacques Vidrine <nectar@FreeBSD.org> To: des@des.no (=?ISO-8859-1?Q?Dag-Erling_Sm=F8rgrav?=) Cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/libexec/rexecd rexecd.c Message-ID: <8e5139a9831f2423984641ee24e2a2a3@FreeBSD.org> In-Reply-To: <86oecur8ie.fsf@xps.des.no> References: <200503271359.j2RDxiF9050487@repoman.freebsd.org> <86oecur8ie.fsf@xps.des.no>
next in thread | previous in thread | raw e-mail | index | archive | help
On Apr 4, 2005, at 8:12 AM, Dag-Erling Sm=F8rgrav wrote: > Jacques Vidrine <nectar@FreeBSD.org> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?8e5139a9831f2423984641ee24e2a2a3>