From b65d12b344a5d81c6060b1c1794afa8858fe234b Mon Sep 17 00:00:00 2001
From: Tim Kientzle <kientzle@acm.org>
Date: Sat, 14 Sep 2024 21:09:34 -0700
Subject: [PATCH] Be more cautious about parsing ISO-9660 timestamps

Some ISO images don't have valid timestamps for the root directory
entry.  Parsing such timestamps can generate nonsensical results,
which in one case showed up as an unexpected overflow on a 32-bit system.

Add some validation logic that can check whether a 7-byte or 17-byte
timestamp is reasonable-looking, and use this to ignore invalid
timestamps in various locations.  This also requires us to be a little
more careful about tracking which timestamps are actually known.

Resolves issue #2329
---
 .../archive_read_support_format_iso9660.c     | 186 ++++++++++++++++--
 libarchive/test/test_read_format_iso_Z.c      |  12 +-
 2 files changed, 177 insertions(+), 21 deletions(-)

diff --git a/libarchive/archive_read_support_format_iso9660.c b/libarchive/archive_read_support_format_iso9660.c
index 056beb5ff..951afb603 100644
--- a/libarchive/archive_read_support_format_iso9660.c
+++ b/libarchive/archive_read_support_format_iso9660.c
@@ -273,7 +273,7 @@ struct file_info {
 	char		 re;		/* Having RRIP "RE" extension.	*/
 	char		 re_descendant;
 	uint64_t	 cl_offset;	/* Having RRIP "CL" extension.	*/
-	int		 birthtime_is_set;
+	int		 time_is_set;	/* Bitmask indicating which times are known */
 	time_t		 birthtime;	/* File created time.		*/
 	time_t		 mtime;		/* File last modified time.	*/
 	time_t		 atime;		/* File last accessed time.	*/
@@ -306,6 +306,11 @@ struct file_info {
 	} rede_files;
 };
 
+#define BIRTHTIME_IS_SET 1
+#define MTIME_IS_SET 2
+#define ATIME_IS_SET 4
+#define CTIME_IS_SET 8
+
 struct heap_queue {
 	struct file_info **files;
 	int		 allocated;
@@ -394,7 +399,9 @@ static void	dump_isodirrec(FILE *, const unsigned char *isodirrec);
 #endif
 static time_t	time_from_tm(struct tm *);
 static time_t	isodate17(const unsigned char *);
+static int	isodate17_valid(const unsigned char *);
 static time_t	isodate7(const unsigned char *);
+static int	isodate7_valid(const unsigned char *);
 static int	isBootRecord(struct iso9660 *, const unsigned char *);
 static int	isVolumePartition(struct iso9660 *, const unsigned char *);
 static int	isVDSetTerminator(struct iso9660 *, const unsigned char *);
@@ -1351,13 +1358,22 @@ archive_read_format_iso9660_read_header(struct archive_read *a,
 	archive_entry_set_uid(entry, file->uid);
 	archive_entry_set_gid(entry, file->gid);
 	archive_entry_set_nlink(entry, file->nlinks);
-	if (file->birthtime_is_set)
+	if ((file->time_is_set & BIRTHTIME_IS_SET))
 		archive_entry_set_birthtime(entry, file->birthtime, 0);
 	else
 		archive_entry_unset_birthtime(entry);
-	archive_entry_set_mtime(entry, file->mtime, 0);
-	archive_entry_set_ctime(entry, file->ctime, 0);
-	archive_entry_set_atime(entry, file->atime, 0);
+	if ((file->time_is_set & MTIME_IS_SET))
+		archive_entry_set_mtime(entry, file->mtime, 0);
+	else
+		archive_entry_unset_mtime(entry);
+	if ((file->time_is_set & CTIME_IS_SET))
+		archive_entry_set_ctime(entry, file->ctime, 0);
+	else
+		archive_entry_unset_ctime(entry);
+	if ((file->time_is_set & ATIME_IS_SET))
+		archive_entry_set_atime(entry, file->atime, 0);
+	else
+		archive_entry_unset_atime(entry);
 	/* N.B.: Rock Ridge supports 64-bit device numbers. */
 	archive_entry_set_rdev(entry, (dev_t)file->rdev);
 	archive_entry_set_size(entry, iso9660->entry_bytes_remaining);
@@ -1898,8 +1914,11 @@ parse_file_info(struct archive_read *a, struct file_info *parent,
 	file->parent = parent;
 	file->offset = offset;
 	file->size = fsize;
-	file->mtime = isodate7(isodirrec + DR_date_offset);
-	file->ctime = file->atime = file->mtime;
+	if (isodate7_valid(isodirrec + DR_date_offset)) {
+		file->time_is_set |= MTIME_IS_SET | ATIME_IS_SET | CTIME_IS_SET;
+		file->mtime = isodate7(isodirrec + DR_date_offset);
+		file->ctime = file->atime = file->mtime;
+	}
 	file->rede_files.first = NULL;
 	file->rede_files.last = &(file->rede_files.first);
 
@@ -2573,51 +2592,73 @@ parse_rockridge_TF1(struct file_info *file, const unsigned char *data,
 		/* Use 17-byte time format. */
 		if ((flag & 1) && data_length >= 17) {
 			/* Create time. */
-			file->birthtime_is_set = 1;
-			file->birthtime = isodate17(data);
+			if (isodate17_valid(data)) {
+				file->time_is_set |= BIRTHTIME_IS_SET;
+				file->birthtime = isodate17(data);
+			}
 			data += 17;
 			data_length -= 17;
 		}
 		if ((flag & 2) && data_length >= 17) {
 			/* Modify time. */
-			file->mtime = isodate17(data);
+			if (isodate17_valid(data)) {
+				file->time_is_set |= MTIME_IS_SET;
+				file->mtime = isodate17(data);
+			}
 			data += 17;
 			data_length -= 17;
 		}
 		if ((flag & 4) && data_length >= 17) {
 			/* Access time. */
-			file->atime = isodate17(data);
+			if (isodate17_valid(data)) {
+				file->time_is_set |= ATIME_IS_SET;
+				file->atime = isodate17(data);
+			}
 			data += 17;
 			data_length -= 17;
 		}
 		if ((flag & 8) && data_length >= 17) {
 			/* Attribute change time. */
-			file->ctime = isodate17(data);
+			if (isodate17_valid(data)) {
+				file->time_is_set |= CTIME_IS_SET;
+				file->ctime = isodate17(data);
+			}
 		}
 	} else {
 		/* Use 7-byte time format. */
 		if ((flag & 1) && data_length >= 7) {
 			/* Create time. */
-			file->birthtime_is_set = 1;
-			file->birthtime = isodate7(data);
+			if (isodate7_valid(data)) {
+				file->time_is_set |= BIRTHTIME_IS_SET;
+				file->birthtime = isodate7(data);
+			}
 			data += 7;
 			data_length -= 7;
 		}
 		if ((flag & 2) && data_length >= 7) {
 			/* Modify time. */
-			file->mtime = isodate7(data);
+			if (isodate7_valid(data)) {
+				file->time_is_set |= MTIME_IS_SET;
+				file->mtime = isodate7(data);
+			}
 			data += 7;
 			data_length -= 7;
 		}
 		if ((flag & 4) && data_length >= 7) {
 			/* Access time. */
-			file->atime = isodate7(data);
+			if (isodate7_valid(data)) {
+				file->time_is_set |= ATIME_IS_SET;
+				file->atime = isodate7(data);
+			}
 			data += 7;
 			data_length -= 7;
 		}
 		if ((flag & 8) && data_length >= 7) {
 			/* Attribute change time. */
-			file->ctime = isodate7(data);
+			if (isodate7_valid(data)) {
+				file->time_is_set |= CTIME_IS_SET;
+				file->ctime = isodate7(data);
+			}
 		}
 	}
 }
@@ -3226,6 +3267,56 @@ isValid733Integer(const unsigned char *p)
 		&& p[3] == p[4]);
 }
 
+static int
+isodate7_valid(const unsigned char *v)
+{
+	int year = v[0];
+	int month = v[1];
+	int day = v[2];
+	int hour = v[3];
+	int minute = v[4];
+	int second = v[5];
+	int gmt_off = (signed char)v[6];
+
+	/* ECMA-119 9.1.5 "If all seven values are zero, it shall mean
+	 * that the date is unspecified" */
+	if (year == 0
+	    && month == 0
+	    && day == 0
+	    && hour == 0
+	    && minute == 0
+	    && second == 0
+	    && gmt_off == 0)
+		return 0;
+	/*
+	 * Sanity-test each individual field
+	 */
+	/* Year can have any value */
+	/* Month must be 1-12 */
+	if (month < 1 || month > 12)
+		return 0;
+	/* Day must be 1-31 */
+	if (day < 1 || day > 31)
+		return 0;
+	/* Hour must be 0-23 */
+	if (hour > 23)
+		return 0;
+	/* Minute must be 0-59 */
+	if (minute > 59)
+		return 0;
+	/* second must be 0-59 according to ECMA-119 9.1.5 */
+	/* BUT: we should probably allow for the time being in UTC, which
+	   allows up to 61 seconds in a minute in certain cases */
+	if (second > 61)
+		return 0;
+	/* Offset from GMT must be -48 to +52 */
+	if (gmt_off < -48 || gmt_off > +52)
+		return 0;
+
+	/* All tests pass, this is OK */
+	return 1;
+}
+
 static time_t
 isodate7(const unsigned char *v)
 {
@@ -3252,6 +3343,67 @@ isodate7(const unsigned char *v)
 	return (t);
 }
 
+static int
+isodate17_valid(const unsigned char *v)
+{
+	/* First 16 bytes are all ASCII digits */
+	for (int i = 0; i < 16; i++) {
+		if (v[i] < '0' || v[i] > '9')
+			return 0;
+	}
+
+	int year = (v[0] - '0') * 1000 + (v[1] - '0') * 100
+		+ (v[2] - '0') * 10 + (v[3] - '0');
+	int month = (v[4] - '0') * 10 + (v[5] - '0');
+	int day = (v[6] - '0') * 10 + (v[7] - '0');
+	int hour = (v[8] - '0') * 10 + (v[9] - '0');
+	int minute = (v[10] - '0') * 10 + (v[11] - '0');
+	int second = (v[12] - '0') * 10 + (v[13] - '0');
+	int hundredths = (v[14] - '0') * 10 + (v[15] - '0');
+	int gmt_off = (signed char)v[16];
+
+	if (year == 0 && month == 0 && day == 0
+	    && hour == 0 && minute == 0 && second == 0
+	    && hundredths == 0 && gmt_off == 0)
+		return 0;
+	/*
+	 * Sanity-test each individual field
+	 */
+
+	/* Year must be 1900-2300 */
+	/* (Not specified in ECMA-119, but these seem
+	   like reasonable limits. */
+	if (year < 1900 || year > 2300)
+		return 0;
+	/* Month must be 1-12 */
+	if (month < 1 || month > 12)
+		return 0;
+	/* Day must be 1-31 */
+	if (day < 1 || day > 31)
+		return 0;
+	/* Hour must be 0-23 */
+	if (hour > 23)
+		return 0;
+	/* Minute must be 0-59 */
+	if (minute > 59)
+		return 0;
+	/* second must be 0-59 according to ECMA-119 9.1.5 */
+	/* BUT: we should probably allow for the time being in UTC, which
+	   allows up to 61 seconds in a minute in certain cases */
+	if (second > 61)
+		return 0;
+	/* Hundredths must be 0-99 */
+	if (hundredths > 99)
+		return 0;
+	/* Offset from GMT must be -48 to +52 */
+	if (gmt_off < -48 || gmt_off > +52)
+		return 0;
+
+	/* All tests pass, this is OK */
+	return 1;
+
+}
+
 static time_t
 isodate17(const unsigned char *v)
 {
diff --git a/libarchive/test/test_read_format_iso_Z.c b/libarchive/test/test_read_format_iso_Z.c
index d07bc1bc8..716552fa3 100644
--- a/libarchive/test/test_read_format_iso_Z.c
+++ b/libarchive/test/test_read_format_iso_Z.c
@@ -93,16 +93,20 @@ test_small(const char *name)
 	assertEqualIntA(a, ARCHIVE_OK,
 	    archive_read_next_header(a, &ae));
 	assertEqualString(".", archive_entry_pathname(ae));
-	assertEqualIntA(a, 3443989665, archive_entry_atime(ae));
-	assertEqualIntA(a, 0, archive_entry_birthtime(ae));
-	assertEqualIntA(a, 3443989665, archive_entry_ctime(ae));
+	assertEqualInt(0, archive_entry_atime_is_set(ae));
+	assertEqualInt(0, archive_entry_atime(ae));
+	assertEqualInt(0, archive_entry_birthtime_is_set(ae));
+	assertEqualInt(0, archive_entry_birthtime(ae));
+	assertEqualInt(0, archive_entry_ctime_is_set(ae));
+	assertEqualInt(0, archive_entry_ctime(ae));
 	assertEqualIntA(a, 0, archive_entry_dev(ae));
 	assertEqualIntA(a, AE_IFDIR, archive_entry_filetype(ae));
 	assertEqualIntA(a, 0, archive_entry_gid(ae));
 	assertEqualStringA(a, NULL, archive_entry_gname(ae));
 	assertEqualIntA(a, 0, archive_entry_ino(ae));
 	assertEqualIntA(a, AE_IFDIR | 0700, archive_entry_mode(ae));
-	assertEqualIntA(a, 3443989665, archive_entry_mtime(ae));
+	assertEqualInt(0, archive_entry_mtime_is_set(ae));
+	assertEqualInt(0, archive_entry_mtime(ae));
 	assertEqualIntA(a, 4, archive_entry_nlink(ae));
 	assertEqualIntA(a, 0700, archive_entry_perm(ae));
 	assertEqualIntA(a, 2048, archive_entry_size(ae));
