pam_unix: fix possible shadow signed overflows

It is possible to trigger signed integer overflows in
check_shadow_expiry if /etc/shadow contains very large values.

Since these values have to be set by a system administrator, it would
already count as a configuration error.

Yet, avoid overflows which would consider accounts which are supposed
to be valid for a veeery long time as already invalid. Also, it would
be undefined behavior for almost all C standards.

Also consider every negative value as invalid, not just -1. The shadow
project has different ways of handling these values, but this approach
is in sync with its lib/isexpired.c implementation.

Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>

1 files changed, 41 insertions, 20 deletions

diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c
index 98f997d5..366f244c 100644
--- a/modules/pam_unix/passverify.c
+++ b/modules/pam_unix/passverify.c
@@ -280,14 +280,29 @@ PAMH_ARG_DECL(int get_pwd_hash,
 	return PAM_SUCCESS;
 }
 
+/*
+ * invariant: 0 <= num1
+ * invariant: 0 <= num2
+ */
+static int
+subtract(long num1, long num2)
+{
+	long value = num1 - num2;
+	if (value < INT_MIN)
+		return INT_MIN;
+	if (value > INT_MAX)
+		return INT_MAX;
+	return (int)value;
+}
+
 PAMH_ARG_DECL(int check_shadow_expiry,
 	struct spwd *spent, int *daysleft)
 {
-	long int curdays;
+	long int curdays, passed;
 	*daysleft = -1;
 	curdays = (long int)(time(NULL) / (60 * 60 * 24));
 	D(("today is %ld, last change %ld", curdays, spent->sp_lstchg));
-	if ((curdays >= spent->sp_expire) && (spent->sp_expire != -1)) {
+	if (spent->sp_expire >= 0 && curdays >= spent->sp_expire) {
 		D(("account expired"));
 		return PAM_ACCT_EXPIRED;
 	}
@@ -302,25 +317,31 @@ PAMH_ARG_DECL(int check_shadow_expiry,
 			 spent->sp_namp);
 		return PAM_SUCCESS;
 	}
-	if ((curdays - spent->sp_lstchg > spent->sp_max)
-	    && (curdays - spent->sp_lstchg > spent->sp_inact)
-	    && (curdays - spent->sp_lstchg > spent->sp_max + spent->sp_inact)
-	    && (spent->sp_max != -1) && (spent->sp_inact != -1)) {
-		*daysleft = (int)((spent->sp_lstchg + spent->sp_max) - curdays);
-		D(("authtok expired"));
-		return PAM_AUTHTOK_EXPIRED;
-	}
-	if ((curdays - spent->sp_lstchg > spent->sp_max) && (spent->sp_max != -1)) {
-		D(("need a new password 2"));
-		return PAM_NEW_AUTHTOK_REQD;
-	}
-	if ((curdays - spent->sp_lstchg > spent->sp_max - spent->sp_warn)
-	    && (spent->sp_max != -1) && (spent->sp_warn != -1)) {
-		*daysleft = (int)((spent->sp_lstchg + spent->sp_max) - curdays);
-		D(("warn before expiry"));
+	passed = curdays - spent->sp_lstchg;
+	if (spent->sp_max >= 0) {
+		if (spent->sp_inact >= 0) {
+			long inact = spent->sp_max < LONG_MAX - spent->sp_inact ?
+			    spent->sp_max + spent->sp_inact : LONG_MAX;
+			if (passed > inact) {
+				*daysleft = subtract(inact, passed);
+				D(("authtok expired"));
+				return PAM_AUTHTOK_EXPIRED;
+			}
+		}
+		if (passed > spent->sp_max) {
+			D(("need a new password 2"));
+			return PAM_NEW_AUTHTOK_REQD;
+		}
+		if (spent->sp_warn >= 0) {
+			long warn = spent->sp_warn > spent->sp_max ? -1 :
+			    spent->sp_max - spent->sp_warn;
+			if (passed > warn) {
+				*daysleft = subtract(spent->sp_max, passed);
+				D(("warn before expiry"));
+			}
+		}
 	}
-	if ((curdays - spent->sp_lstchg < spent->sp_min)
-	    && (spent->sp_min != -1)) {
+	if (spent->sp_min >= 0 && passed < spent->sp_min) {
 		/*
 		 * The last password change was too recent. This error will be ignored
 		 * if no password change is attempted.