From 391353cea6a423ccab1eb2aff764f4ebd34633bf Mon Sep 17 00:00:00 2001 From: Alessandro Mauri Date: Wed, 14 Jul 2021 10:00:07 +0200 Subject: [PATCH] more comment and bug fix commented some stuff and fixed config parser ignoring invalid config keywords --- us.c | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/us.c b/us.c index 68e9605..a6e2e52 100644 --- a/us.c +++ b/us.c @@ -279,10 +279,12 @@ int main(int argc, char *argv[]) {NULL, NULL} }; - if (envflag) { /* clear env */ + /* Copy what has to be copied and then clear the environment, we'll + * make a fresh one later */ + if (envflag) { for (int i = 0; env_keep[i].name; i++) env_keep[i].value = estrdup(getenv(env_keep[i].name)); - environ = NULL; // in place of clearenv + environ = NULL; } for (int i = 0; env_mod[i].name; i++) { @@ -315,7 +317,7 @@ int main(int argc, char *argv[]) } } - // do not override, we might be under more levels of 'us' + /* Do not override, we might be under more levels of 'us' */ err = setenv("US_USER", my_name, 0); errno = 0; @@ -403,7 +405,8 @@ static int authenticate(uid_t uid, char ask) if (!strcmp(p, "x") || *p == '*' || *p == '!') { #if defined(__linux__) - // get exclusive access to shadow + /* Get exclusive access to the shadow file, then read + * this should prevent any race conditions */ if (lckpwdf() == -1) die("lckpwdf"); setspent(); @@ -442,7 +445,9 @@ static int authenticate(uid_t uid, char ask) break; case 0: /* we are still root, drop off permissions before - * disasters happen */ + * disasters happen, also in case askpass fails + * before sending anything to stdout, terminate tha + * main process since it would hang forever */ if (setuid(uid) == -1) { kill(parent, SIGTERM); die("askpass: setuid:"); @@ -479,12 +484,13 @@ static int authenticate(uid_t uid, char ask) fprintf(stderr, "read: %s\n", strerror(errno)); else printf("Password can't be zero length\n"); - // we may have been interrupted, wait askpass before bailing + /* read() may have been interrupted, wait askpass even tough it + * should not be necessary before bailing */ waitpid(-1, NULL, 0); exit(EXIT_FAILURE); } pass[1023] = '\0'; - // remove \n + /* Remove the terminating (if there is) \n in password */ int l = strlen(pass); if (pass[l-1] == '\n') pass[--l] = '\0'; @@ -497,7 +503,7 @@ static int authenticate(uid_t uid, char ask) } char *enc = crypt(pass, hash); - /* clean pass from memory */ + /* Remove password from memory, just to be sure */ memset(pass, 0, 1024); if (strncmp(hash, enc, 1024)) { printf("Authentication failure\n"); @@ -508,6 +514,11 @@ static int authenticate(uid_t uid, char ask) return 0; } +/* user_to_passwd() and group_to_passwd() both receive the user/group name + * in string form, if the sring begins with '#' then the following number + * is interpreted as the uid/gid and used instead of the name. + * Both functions return a pointer to a passwd/group struct stored inside + * the info structure */ static struct passwd* user_to_passwd(const char *user, struct user_info *info) { if (!user) { @@ -560,6 +571,8 @@ static struct group* group_to_grp(const char *group, struct user_info *info) return gr; } +/* die() emalloc() estrdup() and erealloc() are all helper functions, wrappers + * to respective functions that use die() to give a message and then fail */ void die(const char *fmt, ...) { va_list ap; @@ -608,6 +621,8 @@ char *estrdup(const char *s) return r; } +/* Parses the config file and stores the result in a config vector pointed + * by conf of size num */ static int get_config(struct config **conf, int *num) { if (!conf || !num) @@ -692,6 +707,8 @@ static int get_config(struct config **conf, int *num) c.flags |= FLAG_NOPASS; } else if (!strcmp(t, "nolog")) { c.flags |= FLAG_NOLOG; + } else { + die("flag %s not recognized at %d", t, i); } } if (n < 3)