Make user/group-handling-code more robust

As is there is no security issue, but _if_ we end up with a user
or group set to NULL after e.g. ARGEND, we would've hit a null-pointer-
dereference of grp in which is now line 311.

What we want to check instead is if user or group are NULL respectively
and throw an error. Consequently, we can remove the later checks in the
drop root section, as we now guarantee that grp and pwd are not NULL.

Signed-off-by: Laslo Hunhold <dev@frign.de>
This commit is contained in:
Laslo Hunhold 2020-08-09 23:20:06 +02:00
parent b1dca4cf97
commit 660699492f
No known key found for this signature in database
GPG key ID: 69576BD24CFCB980

22
main.c
View file

@ -271,8 +271,8 @@ main(int argc, char *argv[])
}
if (udsname && (!access(udsname, F_OK) || errno != ENOENT)) {
die("UNIX-domain socket: %s", errno ?
strerror(errno) : "file exists");
die("UNIX-domain socket '%s': %s", udsname, errno ?
strerror(errno) : "File exists");
}
/* compile and check the supplied vhost regexes */
@ -292,14 +292,14 @@ main(int argc, char *argv[])
/* validate user and group */
errno = 0;
if (user && !(pwd = getpwnam(user))) {
die("getpwnam '%s': %s", user, errno ? strerror(errno) :
"Entry not found");
if (!user || !(pwd = getpwnam(user))) {
die("getpwnam '%s': %s", user ? user : "null",
errno ? strerror(errno) : "Entry not found");
}
errno = 0;
if (group && !(grp = getgrnam(group))) {
die("getgrnam '%s': %s", group, errno ? strerror(errno) :
"Entry not found");
if (!group || !(grp = getgrnam(group))) {
die("getgrnam '%s': %s", group ? group : "null",
errno ? strerror(errno) : "Entry not found");
}
/* open a new process group */
@ -337,13 +337,13 @@ main(int argc, char *argv[])
}
/* drop root */
if (grp && setgroups(1, &(grp->gr_gid)) < 0) {
if (setgroups(1, &(grp->gr_gid)) < 0) {
die("setgroups:");
}
if (grp && setgid(grp->gr_gid) < 0) {
if (setgid(grp->gr_gid) < 0) {
die("setgid:");
}
if (pwd && setuid(pwd->pw_uid) < 0) {
if (setuid(pwd->pw_uid) < 0) {
die("setuid:");
}