set*id() return values not checked in pop3d and comsatd

--000000000000e16c2006031ad8a0
Content-Type: multipart/alternative; boundary="000000000000e16c1e06031ad89e"

--000000000000e16c1e06031ad89e
Content-Type: text/plain; charset="UTF-8"

Hi,

Several set*id() family function return values are not checked in pop3d and
comsatd code:

pop3d/user.c:

     21 int
     22 pop3d_begin_session ()
     23 {
    [...]
     39
     40   if (auth_data->change_uid)
     41     setuid (auth_data->uid);

pop3d/popauth.c:

    469 int
    470 action_create (void)
    471 {
    [...]
    479
    480   /* Make sure we have proper privileges if popauth is setuid */
    481   setuid (getuid ());

comsat/comsat.c:

    526 int
    527 change_user (const char *user)
    528 {
    529   struct passwd *pw;
    530
    531   pw = getpwnam (user);
    532   if (!pw)
    533     {
    534       mu_diag_output (MU_DIAG_CRIT, _("no such user: %s"), user);
    535       return 1;
    536     }
    537
    538   setgid (pw->pw_gid);
    539   setuid (pw->pw_uid);
    540   chdir (pw->pw_dir);
    541   username = user;
    542   return 0;
    543 }

There are cases where set*id() functions can fail.

man 2 setuid():

RETURN VALUE
       On success, zero is returned.  On error, -1 is returned, and errno
is set to indicate the error.

       Note: there are cases where setuid() can fail even when the caller
is UID 0; it is a grave security error to omit checking for a failure
return from setuid().


The above code could be abused in different ways to trigger such failures,
potentially remotely in the case of pop3d and comsatd. That could lead to
either privilege
escalations or user impersonation depending on the occurrence.

I believe the attached patch mitigates the issue, let me know if that suits
you.

Regards,

-- 
Jeffrey BENCTEUX

--000000000000e16c1e06031ad89e
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><div>Hi,</div><div><br></div><div>Several=
 set*id() family function return values are not checked in pop3d and comsat=
d code:</div><div><br></div><div>pop3d/user.c:</div><div><br></div><div>=C2=
=A0 =C2=A0 =C2=A021 int</div><div>=C2=A0 =C2=A0 =C2=A022 pop3d_begin_sessio=
n ()</div><div>=C2=A0 =C2=A0 =C2=A023 {</div><div>=C2=A0 =C2=A0 [...]</div>=
<div>=C2=A0 =C2=A0 =C2=A039=C2=A0 =C2=A0</div><div>=C2=A0 =C2=A0 =C2=A040=
=C2=A0 =C2=A0if (auth_data-&gt;change_uid)</div><div>=C2=A0 =C2=A0 =C2=A041=
=C2=A0 =C2=A0 =C2=A0setuid (auth_data-&gt;uid);</div><div><br></div><div>po=
p3d/popauth.c:=C2=A0</div><div><br></div><div>=C2=A0 =C2=A0 469 int</div><d=
iv>=C2=A0 =C2=A0 470 action_create (void)</div><div>=C2=A0 =C2=A0 471 {</di=
v><div>=C2=A0 =C2=A0 [...]</div><div>=C2=A0 =C2=A0 479=C2=A0 =C2=A0</div><d=
iv>=C2=A0 =C2=A0 480=C2=A0 =C2=A0/* Make sure we have proper privileges if =
popauth is setuid */</div><div>=C2=A0 =C2=A0 481=C2=A0 =C2=A0setuid (getuid=
 ());</div><div><br></div><div>comsat/comsat.c:</div><div><br></div><div>=
=C2=A0 =C2=A0 526 int</div><div>=C2=A0 =C2=A0 527 change_user (const char *=
user)</div><div>=C2=A0 =C2=A0 528 {</div><div>=C2=A0 =C2=A0 529=C2=A0 =C2=
=A0struct passwd *pw;</div><div>=C2=A0 =C2=A0 530=C2=A0</div><div>=C2=A0 =
=C2=A0 531=C2=A0 =C2=A0pw =3D getpwnam (user);</div><div>=C2=A0 =C2=A0 532=
=C2=A0 =C2=A0if (!pw)</div><div>=C2=A0 =C2=A0 533=C2=A0 =C2=A0 =C2=A0{</div=
><div>=C2=A0 =C2=A0 534=C2=A0 =C2=A0 =C2=A0 =C2=A0mu_diag_output (MU_DIAG_C=
RIT, _(&quot;no such user: %s&quot;), user);</div><div>=C2=A0 =C2=A0 535=C2=
=A0 =C2=A0 =C2=A0 =C2=A0return 1;</div><div>=C2=A0 =C2=A0 536=C2=A0 =C2=A0 =
=C2=A0}</div><div>=C2=A0 =C2=A0 537=C2=A0</div><div>=C2=A0 =C2=A0 538=C2=A0=
 =C2=A0setgid (pw-&gt;pw_gid);</div><div>=C2=A0 =C2=A0 539=C2=A0 =C2=A0setu=
id (pw-&gt;pw_uid);</div><div>=C2=A0 =C2=A0 540=C2=A0 =C2=A0chdir (pw-&gt;p=
w_dir);</div><div>=C2=A0 =C2=A0 541=C2=A0 =C2=A0username =3D user;</div><di=
v>=C2=A0 =C2=A0 542=C2=A0 =C2=A0return 0;</div><div>=C2=A0 =C2=A0 543 }</di=
v><div><br></div><div>There are cases where set*id() functions can fail.</d=
iv><div><br></div><div>man 2 setuid():</div><div><br></div><div>RETURN VALU=
E</div><div>=C2=A0 =C2=A0 =C2=A0 =C2=A0On success, zero is returned.=C2=A0 =
On error, -1 is returned, and errno is set to indicate the error.</div><div=
><br></div><div>=C2=A0 =C2=A0 =C2=A0 =C2=A0Note: there are cases where setu=
id() can fail even when the caller is UID 0; it is a grave security error t=
o omit checking for a failure return from setuid().</div><div><br></div><di=
v><br></div><div>The above code could be abused in different ways to trigge=
r such failures, potentially remotely in the case of pop3d and comsatd. Tha=
t could lead to either privilege</div><div>escalations or user impersonatio=
n depending on the occurrence.</div><div><br></div><div>I believe the attac=
hed patch mitigates the issue, let me know if that suits you.</div><div><br=
></div><div>Regards,</div><div><br></div><div><div dir=3D"ltr" class=3D"gma=
il_signature"><div dir=3D"ltr">-- <br>Jeffrey BENCTEUX<br></div></div></div=
></div></div>

--000000000000e16c1e06031ad89e--

--000000000000e16c2006031ad8a0
Content-Type: application/octet-stream; 
	name="0001-pop3-comsat-fix-missing-checks-for-set-id-return-val.patch"
Content-Disposition: attachment; 
	filename="0001-pop3-comsat-fix-missing-checks-for-set-id-return-val.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_lley2fz00>
X-Attachment-Id: f_lley2fz00

RnJvbSA4NjkxNTJlMzllMjcyMGFhYjg2ZTFkNGQ5NzFiZmM3YTI5Y2Q4YTg5IE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBKZWZmcmV5IEJlbmN0ZXV4IDxqZWZmYmVuY3RldXhAZ21haWwu
Y29tPgpEYXRlOiBUaHUsIDE3IEF1ZyAyMDIzIDExOjA2OjQxICswMjAwClN1YmplY3Q6IFtQQVRD
SF0gcG9wMywgY29tc2F0OiBmaXggbWlzc2luZyBjaGVja3MgZm9yIHNldCppZCgpIHJldHVybiB2
YWx1ZXMKClNldmVyYWwgc2V0KmlkKCkgZmFtaWx5IGZ1bmN0aW9uIGNhbGxzIHJldHVybiB2YWx1
ZXMgd2VyZQp1bmNoZWNrZWQgaW4gcG9wMyBhbmQgY29tc2F0IGxlYWRpbmcgdG8gcG90ZW50aWFs
IHByaXZpbGVnZQplc2NhbGF0aW9ucyBvciB1c2VyIGltcGVyc29uYXRpb25zLgoKU2lnbmVkLW9m
Zi1ieTogSmVmZnJleSBCZW5jdGV1eCA8amVmZmJlbmN0ZXV4QGdtYWlsLmNvbT4KLS0tCiBjb21z
YXQvY29tc2F0LmMgfCAxMiArKysrKysrKysrLS0KIHBvcDNkL3BvcGF1dGguYyB8ICA2ICsrKysr
LQogcG9wM2QvdXNlci5jICAgIHwgMTMgKysrKysrKysrKy0tLQogMyBmaWxlcyBjaGFuZ2VkLCAy
NSBpbnNlcnRpb25zKCspLCA2IGRlbGV0aW9ucygtKQoKZGlmZiAtLWdpdCBhL2NvbXNhdC9jb21z
YXQuYyBiL2NvbXNhdC9jb21zYXQuYwppbmRleCBiZWJlMTMzNmIuLjAwM2U2OTIzMyAxMDA2NDQK
LS0tIGEvY29tc2F0L2NvbXNhdC5jCisrKyBiL2NvbXNhdC9jb21zYXQuYwpAQCAtNTM0LDkgKzUz
NCwxNyBAQCBjaGFuZ2VfdXNlciAoY29uc3QgY2hhciAqdXNlcikKICAgICAgIG11X2RpYWdfb3V0
cHV0IChNVV9ESUFHX0NSSVQsIF8oIm5vIHN1Y2ggdXNlcjogJXMiKSwgdXNlcik7CiAgICAgICBy
ZXR1cm4gMTsKICAgICB9CisgIGlmIChzZXRnaWQgKHB3LT5wd19naWQpID09IC0xKQorICAgIHsK
KyAgICAgIG11X2RpYWdfb3V0cHV0IChNVV9ESUFHX0NSSVQsIF8oInNldGdpZCBmYWlsZWQgZm9y
IHVzZXI6ICVzIiksIHVzZXIpOworICAgICAgcmV0dXJuIDE7CisgICAgfQorICBpZiAoc2V0dWlk
IChwdy0+cHdfdWlkKSA9PSAtMSkKKyAgICB7CisgICAgICBtdV9kaWFnX291dHB1dCAoTVVfRElB
R19DUklULCBfKCJzZXR1aWQgZmFpbGVkIGZvciB1c2VyOiAlcyIpLCB1c2VyKTsKKyAgICAgIHJl
dHVybiAxOworICAgIH0KIAotICBzZXRnaWQgKHB3LT5wd19naWQpOwotICBzZXR1aWQgKHB3LT5w
d191aWQpOwogICBjaGRpciAocHctPnB3X2Rpcik7CiAgIHVzZXJuYW1lID0gdXNlcjsKICAgcmV0
dXJuIDA7CmRpZmYgLS1naXQgYS9wb3AzZC9wb3BhdXRoLmMgYi9wb3AzZC9wb3BhdXRoLmMKaW5k
ZXggYmFhNDZjOTczLi4zOGRiZGRhZGIgMTAwNjQ0Ci0tLSBhL3BvcDNkL3BvcGF1dGguYworKysg
Yi9wb3AzZC9wb3BhdXRoLmMKQEAgLTQ3OCw3ICs0NzgsMTEgQEAgYWN0aW9uX2NyZWF0ZSAodm9p
ZCkKICAgaW50IGxpbmUgPSAwOwogICAKICAgLyogTWFrZSBzdXJlIHdlIGhhdmUgcHJvcGVyIHBy
aXZpbGVnZXMgaWYgcG9wYXV0aCBpcyBzZXR1aWQgKi8KLSAgc2V0dWlkIChnZXR1aWQgKCkpOwor
ICBpZiAoc2V0dWlkIChnZXR1aWQgKCkpID09IC0xKQorICAgIHsKKyAgICAgIG11X2Vycm9yKF8o
IkRyb3BwaW5nIHByaXZpbGVnZXMgZmFpbGVkIikpOworICAgICAgcmV0dXJuIDE7CisgICAgfQog
ICAKICAgaWYgKGlucHV0X25hbWUpCiAgICAgewpkaWZmIC0tZ2l0IGEvcG9wM2QvdXNlci5jIGIv
cG9wM2QvdXNlci5jCmluZGV4IDM4NzQxNmY3Ny4uNjcxZDM4M2I4IDEwMDY0NAotLS0gYS9wb3Az
ZC91c2VyLmMKKysrIGIvcG9wM2QvdXNlci5jCkBAIC0zNiwxMCArMzYsMTcgQEAgcG9wM2RfYmVn
aW5fc2Vzc2lvbiAoKQogICAgICAgbXVfYXV0aF9kYXRhX2Rlc3Ryb3kgKCZhdXRoX2RhdGEpOwog
ICAgICAgcmV0dXJuIEVSUl9MT0dJTl9ERUxBWTsKICAgICB9Ci0gIAorCiAgIGlmIChhdXRoX2Rh
dGEtPmNoYW5nZV91aWQpCi0gICAgc2V0dWlkIChhdXRoX2RhdGEtPnVpZCk7Ci0gIAorICAgIHsK
KyAgICAgIGlmIChzZXR1aWQgKGF1dGhfZGF0YS0+dWlkKSA9PSAtMSkKKyAgICAgICAgeworICAg
ICAgICAgIG11X2RpYWdfb3V0cHV0IChNVV9ESUFHX0lORk8sCisJCQkgIF8oInNldHVpZCBmYWls
ZWQgZm9yIHVzZXIgYCVzJyIpLCBhdXRoX2RhdGEtPm5hbWUpOworICAgICAgICAgIHJldHVybiBF
UlJfQkFEX0xPR0lOOworICAgICAgICB9CisgICAgfQorCiAgIGlmIChtYW5sb2NrX29wZW5fbWFp
bGJveCAoJm1ib3gsIGF1dGhfZGF0YS0+bWFpbGJveCwgMCwKIAkJCSAgICBNVV9TVFJFQU1fQ1JF
QVQgfCBNVV9TVFJFQU1fUkRXUikpCiAgICAgewotLSAKMi4zNS4xCgo=
--000000000000e16c2006031ad8a0--