From 247a869ccdc442d39cd1d0d2a53014e3af26516c Mon Sep 17 00:00:00 2001 From: =?utf8?q?Christian=20G=C3=B6ttsche?= Date: Tue, 28 Feb 2023 17:24:22 +0100 Subject: [PATCH] faillog: check for overflows Check for arithmetic overflows when computing offsets to avoid file corruptions for huge UIDs. Refactor the file lookup into a separate function. --- src/faillog.c | 140 +++++++++++++++++++------------------------------- 1 file changed, 53 insertions(+), 87 deletions(-) diff --git a/src/faillog.c b/src/faillog.c index ab60775a4..874528d88 100644 --- a/src/faillog.c +++ b/src/faillog.c @@ -84,34 +84,36 @@ usage (int status) exit (status); } -static void print_one (/*@null@*/const struct passwd *pw, bool force) +/* + * Looks up the offset in the faillog file for the given uid. + * Returns -1 on error. + */ +static off_t lookup_faillog(struct faillog *fl, uid_t uid) { - static bool once = false; - struct tm *tm; - off_t offset; - struct faillog fl; - time_t now; - char *cp; - char ptime[80]; - - if (NULL == pw) { - return; + off_t offset, size; + + /* Ensure multiplication does not overflow and retrieving a wrong entry */ + if (__builtin_mul_overflow(uid, sizeof(*fl), &offset)) { + fprintf(stderr, + _("%s: Failed to get the entry for UID %lu\n"), + Prog, (unsigned long int)uid); + return -1; } - offset = (off_t) pw->pw_uid * sizeof (fl); - if (offset + sizeof (fl) <= statbuf.st_size) { + if (!__builtin_add_overflow(offset, sizeof(*fl), &size) + && size <= statbuf.st_size) { /* fseeko errors are not really relevant for us. */ - int err = fseeko (fail, offset, SEEK_SET); - assert (0 == err); + int err = fseeko(fail, offset, SEEK_SET); + assert(0 == err); /* faillog is a sparse file. Even if no entries were * entered for this user, which should be able to get the * empty entry in this case. */ - if (fread (&fl, sizeof (fl), 1, fail) != 1) { - fprintf (stderr, - _("%s: Failed to get the entry for UID %lu\n"), - Prog, (unsigned long int)pw->pw_uid); - return; + if (fread(fl, sizeof(*fl), 1, fail) != 1) { + fprintf(stderr, + _("%s: Failed to get the entry for UID %lu\n"), + Prog, (unsigned long int)uid); + return -1; } } else { /* Outsize of the faillog file. @@ -119,7 +121,27 @@ static void print_one (/*@null@*/const struct passwd *pw, bool force) * as if we were reading an non existing entry in the * sparse faillog file). */ - memzero (&fl, sizeof (fl)); + memzero(fl, sizeof(*fl)); + } + + return offset; +} + +static void print_one (/*@null@*/const struct passwd *pw, bool force) +{ + static bool once = false; + struct tm *tm; + struct faillog fl; + time_t now; + char *cp; + char ptime[80]; + + if (NULL == pw) { + return; + } + + if (lookup_faillog(&fl, pw->pw_uid) < 0) { + return; } /* Nothing to report */ @@ -200,28 +222,10 @@ static bool reset_one (uid_t uid) off_t offset; struct faillog fl; - offset = (off_t) uid * sizeof (fl); - if (offset + sizeof (fl) <= statbuf.st_size) { - /* fseeko errors are not really relevant for us. */ - int err = fseeko (fail, offset, SEEK_SET); - assert (0 == err); - /* faillog is a sparse file. Even if no entries were - * entered for this user, which should be able to get the - * empty entry in this case. - */ - if (fread (&fl, sizeof (fl), 1, fail) != 1) { - fprintf (stderr, - _("%s: Failed to get the entry for UID %lu\n"), - Prog, (unsigned long int)uid); - return true; - } - } else { - /* Outsize of the faillog file. - * Behave as if there were a missing entry (same behavior - * as if we were reading an non existing entry in the - * sparse faillog file). - */ - memzero (&fl, sizeof (fl)); + offset = lookup_faillog(&fl, uid); + if (offset < 0) { + /* failure */ + return true; } if (0 == fl.fail_cnt) { @@ -314,28 +318,9 @@ static bool setmax_one (uid_t uid, short max) off_t offset; struct faillog fl; - offset = (off_t) uid * sizeof (fl); - if (offset + sizeof (fl) <= statbuf.st_size) { - /* fseeko errors are not really relevant for us. */ - int err = fseeko (fail, offset, SEEK_SET); - assert (0 == err); - /* faillog is a sparse file. Even if no entries were - * entered for this user, which should be able to get the - * empty entry in this case. - */ - if (fread (&fl, sizeof (fl), 1, fail) != 1) { - fprintf (stderr, - _("%s: Failed to get the entry for UID %lu\n"), - Prog, (unsigned long int)uid); - return true; - } - } else { - /* Outsize of the faillog file. - * Behave as if there were a missing entry (same behavior - * as if we were reading an non existing entry in the - * sparse faillog file). - */ - memzero (&fl, sizeof (fl)); + offset = lookup_faillog(&fl, uid); + if (offset < 0) { + return true; } if (max == fl.fail_max) { @@ -431,28 +416,9 @@ static bool set_locktime_one (uid_t uid, long locktime) off_t offset; struct faillog fl; - offset = (off_t) uid * sizeof (fl); - if (offset + sizeof (fl) <= statbuf.st_size) { - /* fseeko errors are not really relevant for us. */ - int err = fseeko (fail, offset, SEEK_SET); - assert (0 == err); - /* faillog is a sparse file. Even if no entries were - * entered for this user, which should be able to get the - * empty entry in this case. - */ - if (fread (&fl, sizeof (fl), 1, fail) != 1) { - fprintf (stderr, - _("%s: Failed to get the entry for UID %lu\n"), - Prog, (unsigned long int)uid); - return true; - } - } else { - /* Outsize of the faillog file. - * Behave as if there were a missing entry (same behavior - * as if we were reading an non existing entry in the - * sparse faillog file). - */ - memzero (&fl, sizeof (fl)); + offset = lookup_faillog(&fl, uid); + if (offset < 0) { + return true; } if (locktime == fl.fail_locktime) { -- 2.47.2