summaryrefslogtreecommitdiff
blob: 2c5bcc277364d04e5f17e39f371442aa64520325 (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
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));