core icon indicating copy to clipboard operation
core copied to clipboard

Update imap-date.c to check if number of days is correct

Open ftaurino opened this issue 1 year ago • 6 comments

Add some basic checks to ensure tm_mday values is correct for the month entered, especially for doveadm search conditions

ftaurino avatar May 06 '23 14:05 ftaurino

Looks like the following utc_mktime() already fully validates all the dates. However, we're mostly just ignoring the results and assuming the timestamp is too large or too small. I think something like the patch below would work better, except the 2038 check needs to be better somehow.

diff --git a/src/lib-imap/imap-date.c b/src/lib-imap/imap-date.c
index 2f04cabcce..885123ae4c 100644
--- a/src/lib-imap/imap-date.c
+++ b/src/lib-imap/imap-date.c
@@ -79,11 +79,11 @@ static const char *imap_parse_date_internal(const char *str, struct tm *tm)
 	return str;
 }
 
-static bool imap_mktime(struct tm *tm, time_t *time_r)
+static int imap_mktime(struct tm *tm, time_t *time_r)
 {
 	*time_r = utc_mktime(tm);
 	if (*time_r != (time_t)-1)
-		return TRUE;
+		return 1;
 
 	/* the date is outside valid range for time_t. it might still be
 	   technically valid though, so try to handle this case.
@@ -96,7 +96,8 @@ static bool imap_mktime(struct tm *tm, time_t *time_r)
 #else
 		*time_r = 0;
 #endif
-	} else {
+		return 0;
+	} else if (tm->tm_year >= 2038-1970) {
 		/* too high. return the highest allowed value.
 		   we shouldn't get here with 64bit time_t,
 		   but handle that anyway. */
@@ -105,8 +106,10 @@ static bool imap_mktime(struct tm *tm, time_t *time_r)
 #else
 		*time_r = (1UL << TIME_T_MAX_BITS) - 1;
 #endif
+		return 0;
+	} else {
+		return -1;
 	}
-	return FALSE;
 }
 
 bool imap_parse_date(const char *str, time_t *timestamp_r)
@@ -118,7 +121,8 @@ bool imap_parse_date(const char *str, time_t *timestamp_r)
 		return FALSE;
 
 	tm.tm_isdst = -1;
-	(void)imap_mktime(&tm, timestamp_r);
+	if (imap_mktime(&tm, timestamp_r) < 0)
+		return FALSE;
 	return TRUE;
 }
 
@@ -126,6 +130,7 @@ bool imap_parse_datetime(const char *str, time_t *timestamp_r,
 			 int *timezone_offset_r)
 {
 	struct tm tm;
+	int ret;
 
 	str = imap_parse_date_internal(str, &tm);
 	if (str == NULL)
@@ -157,9 +162,9 @@ bool imap_parse_datetime(const char *str, time_t *timestamp_r,
 	*timezone_offset_r = parse_timezone(str);
 
 	tm.tm_isdst = -1;
-	if (imap_mktime(&tm, timestamp_r))
+	if ((ret = imap_mktime(&tm, timestamp_r)) > 0)
 		*timestamp_r -= *timezone_offset_r * 60;
-	return TRUE;
+	return ret >= 0;
 }
 
 static void imap_to_date_tm(char buf[11], const struct tm *tm)

sirainen avatar May 07 '23 20:05 sirainen

Maybe the check is needed only with 32bit time_t, and with 64bit time_t just handle all (time_t)-1 return values as invalid.

sirainen avatar May 07 '23 20:05 sirainen

Without this check, you can search messages with "doveadm" with dates like "32-Jan-2022" or "31-Apr-2023", with very strange results. This simple check just prevent this situation, before calling imap_mktime.

ftaurino avatar May 08 '23 06:05 ftaurino

But you get better correctness with the patch I added, e.g. you can't use "29-Feb-2023". Just would need to replace tm->tm_year >= 2038-1970 somehow better.

sirainen avatar May 08 '23 07:05 sirainen

Perhaps we can consider this basic check a date input validation, working both on 32 and 64 bit systems and also after 2038.

ftaurino avatar May 08 '23 11:05 ftaurino

It's unnecessary to add such basic validation though if it can be done perfectly instead.. I did a bit improved patch that seems to catch all the cases correctly with 32bit and 64bit time_t. Tracking internally in DOP-3196.

sirainen avatar May 13 '23 21:05 sirainen