[hackers] [quark] Make user/group-handling-code more robust || Laslo Hunhold

From: <git_AT_suckless.org>
Date: Sun, 9 Aug 2020 23:22:38 +0200 (CEST)

commit 660699492fb2275335d24fb26ec3c6529f623af0
Author: Laslo Hunhold <dev_AT_frign.de>
AuthorDate: Sun Aug 9 23:20:06 2020 +0200
Commit: Laslo Hunhold <dev_AT_frign.de>
CommitDate: Sun Aug 9 23:20:06 2020 +0200

    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_AT_frign.de>

diff --git a/main.c b/main.c
index 92b8214..0542fab 100644
--- a/main.c
+++ b/main.c
_AT_@ -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 */
_AT_@ -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 */
_AT_@ -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:");
                 }
 
Received on Sun Aug 09 2020 - 23:22:38 CEST