Date: Mon, 01 Oct 2012 04:07:33 -0700 From: Xin Li <delphij@delphij.net> To: Erik Cederstrand <erik@cederstrand.dk> Cc: "freebsd-security@freebsd.org" <freebsd-security@freebsd.org> Subject: Re: Opinion on checking return value of setuid(getuid())? Message-ID: <50697975.4070609@delphij.net> In-Reply-To: <9DD86238-51C8-4F38-B7EB-BD773039888B@cederstrand.dk> References: <9DD86238-51C8-4F38-B7EB-BD773039888B@cederstrand.dk>
next in thread | previous in thread | raw e-mail | index | archive | help
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 On 10/1/12 3:31 AM, Erik Cederstrand wrote: > I'm looking through the clang analyzer reports and found this one: > http://scan.freebsd.your.org/freebsd-head/sbin.ping/2012-09-30-amd64/report-R9ZgC6.html#EndPath > > > It's complaining that, if setuid() fails for some reason, the > process will continue with root privileges because the process is > suid root. > > At first glance, it seems unnecessary to check the return value of > "setuid(getuid())" since the user should always be able to drop > privileges to itself. So I filed this bug with LLVM: > http://llvm.org/bugs/show_bug.cgi?id=13979 > > It turns out that setuid() *may* fail if the user hits its process > limit. Apparently FreeBSD doesn't check the limit in the specific > setuid(getuid()) case (I can't find the code anywhere right now) > so this is not an issue, but Linux does. However, if FreeBSD > decides to change the setuid() implementation at some point, the > issue may surface again. I didn't follow the idea -- In Linux's kernel/sys.c: SYSCALL_DEFINE1(setuid, uid_t, uid) { (...) kuid = make_kuid(ns, uid); (...) if (nsown_capable(CAP_SETUID)) { new->suid = new->uid = kuid; if (!uid_eq(kuid, old->uid)) { // <-- 1 retval = set_user(new); // <-- check done here if (retval < 0) goto error; } How can the check be even reached in setuid(getuid()) case? It's also conflict with intuition by the way -- we are not changing ownership of the process, thus the process number should not change... Cheers, -----BEGIN PGP SIGNATURE----- Version: GnuPG/MacGPG2 v2.0.17 (Darwin) iQEcBAEBCAAGBQJQaXl0AAoJEG80Jeu8UPuz22AIAIBhAdEscXjcsQR06qzFntn4 lVVLzlPH+KdgUezbE5uMWbtNj0Az7ny66QQ2ocgh5KK8bc5i1486T9+32k6X7Cft gxE7tpPGkrb6uT62TV4Z5TkJ3NLfqQ6pABiYFONUS72Zy2zPE9stq5X4XrySXlTh Oft6hpLK5qtxucD7RUKrj8Ofw6kugKm7+KDXqQUU2CuEkCZZUiY1KarJK1fyPHF7 9APaaWyWZt6yMj3qn/2btmR4GZoZMQdfUqe8EIhpxGdKseB81FIdHfDo2bzDGRcx jUUIbrFxLTypjXws2IPneHYaKpLfs5RWT6yKPRkdKIkfQYTeJMb0MjlD6q7acWo= =hknO -----END PGP SIGNATURE-----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?50697975.4070609>