[tz] [PROPOSED] Be more generous when dealing with v1 data

[Paul Eggert]

* NEWS: Mention this.
* localtime.c (tzloadbody): In version 2 and later files, do not
attempt to validate the version 1 header and data block; instead,
ignore them except for the purpose of skipping over them, as per a
recommendation in RFC 8536 section 4.
Also, when the TZif file is version 1 and we break out near the
end of the loop, do so only after altering nread and memmoving any
later data; this avoids having later code attempt to parse the
version 1 header as if it were a TZ string.
---
 NEWS        |  6 +++++
 localtime.c | 71 +++++++++++++++++++++++++++++++----------------------
 2 files changed, 47 insertions(+), 30 deletions(-)

diff --git a/NEWS b/NEWS
index d4a9823..0f684f7 100644
--- a/NEWS
+++ b/NEWS
@@ -14,6 +14,12 @@ Unreleased, experimental changes
     Fix bug when 32-bit time_t code reads malformed 64-bit TZif data.
     (Problem reported by Christos Zoulas.)
 
+    When reading a version 2 or later TZif file, the TZif reader now
+    validates the version 1 header and data block only enough to skip
+    over them, as recommended by RFC 8536 section 4.  Also, the TZif
+    reader no longer mistakenly attempts to parse a version 1 TZIf
+    file header as a TZ string.
+
 
 Release 2021e - 2021-10-21 18:41:00 -0700
 
diff --git a/localtime.c b/localtime.c
index 6d736b2..39ef6e3 100644
--- a/localtime.c
+++ b/localtime.c
@@ -434,35 +434,45 @@ tzloadbody(char const *name, struct state *sp, bool doextend,
 	if (close(fid) < 0)
 	  return errno;
 	for (stored = 4; stored <= 8; stored *= 2) {
-		int_fast32_t ttisstdcnt = detzcode(up->tzhead.tzh_ttisstdcnt);
-		int_fast32_t ttisutcnt = detzcode(up->tzhead.tzh_ttisutcnt);
-		int_fast64_t prevtr = -1;
-		int_fast32_t prevcorr;
-		int_fast32_t leapcnt = detzcode(up->tzhead.tzh_leapcnt);
-		int_fast32_t timecnt = detzcode(up->tzhead.tzh_timecnt);
-		int_fast32_t typecnt = detzcode(up->tzhead.tzh_typecnt);
-		int_fast32_t charcnt = detzcode(up->tzhead.tzh_charcnt);
-		char const *p = up->buf + tzheadsize;
-		/* Although tzfile(5) currently requires typecnt to be nonzero,
-		   support future formats that may allow zero typecnt
-		   in files that have a TZ string and no transitions.  */
-		if (! (0 <= leapcnt && leapcnt < TZ_MAX_LEAPS
-		       && 0 <= typecnt && typecnt < TZ_MAX_TYPES
-		       && 0 <= timecnt && timecnt < TZ_MAX_TIMES
-		       && 0 <= charcnt && charcnt < TZ_MAX_CHARS
-		       && (ttisstdcnt == typecnt || ttisstdcnt == 0)
-		       && (ttisutcnt == typecnt || ttisutcnt == 0)))
-		  return EINVAL;
-		if (nread
-		    < (tzheadsize		/* struct tzhead */
-		       + timecnt * stored	/* ats */
+	    char version = up->tzhead.tzh_version[0];
+	    bool skip_datablock = stored == 4 && version;
+	    int_fast32_t datablock_size;
+	    int_fast32_t ttisstdcnt = detzcode(up->tzhead.tzh_ttisstdcnt);
+	    int_fast32_t ttisutcnt = detzcode(up->tzhead.tzh_ttisutcnt);
+	    int_fast64_t prevtr = -1;
+	    int_fast32_t prevcorr;
+	    int_fast32_t leapcnt = detzcode(up->tzhead.tzh_leapcnt);
+	    int_fast32_t timecnt = detzcode(up->tzhead.tzh_timecnt);
+	    int_fast32_t typecnt = detzcode(up->tzhead.tzh_typecnt);
+	    int_fast32_t charcnt = detzcode(up->tzhead.tzh_charcnt);
+	    char const *p = up->buf + tzheadsize;
+	    /* Although tzfile(5) currently requires typecnt to be nonzero,
+	       support future formats that may allow zero typecnt
+	       in files that have a TZ string and no transitions.  */
+	    if (! (0 <= leapcnt && leapcnt < TZ_MAX_LEAPS
+		   && 0 <= typecnt && typecnt < TZ_MAX_TYPES
+		   && 0 <= timecnt && timecnt < TZ_MAX_TIMES
+		   && 0 <= charcnt && charcnt < TZ_MAX_CHARS
+		   && 0 <= ttisstdcnt && ttisstdcnt < TZ_MAX_TYPES
+		   && 0 <= ttisutcnt && ttisutcnt < TZ_MAX_TYPES))
+	      return EINVAL;
+	    datablock_size
+		    = (timecnt * stored		/* ats */
 		       + timecnt		/* types */
 		       + typecnt * 6		/* ttinfos */
 		       + charcnt		/* chars */
 		       + leapcnt * (stored + 4)	/* lsinfos */
 		       + ttisstdcnt		/* ttisstds */
-		       + ttisutcnt))		/* ttisuts */
+		       + ttisutcnt);		/* ttisuts */
+	    if (nread < tzheadsize + datablock_size)
+	      return EINVAL;
+	    if (skip_datablock)
+		p += datablock_size;
+	    else {
+		if (! ((ttisstdcnt == typecnt || ttisstdcnt == 0)
+		       && (ttisutcnt == typecnt || ttisutcnt == 0)))
 		  return EINVAL;
+
 		sp->leapcnt = leapcnt;
 		sp->timecnt = timecnt;
 		sp->typecnt = typecnt;
@@ -578,13 +588,14 @@ tzloadbody(char const *name, struct state *sp, bool doextend,
 				ttisp->tt_ttisut = *p++;
 			}
 		}
-		/*
-		** If this is an old file, we're done.
-		*/
-		if (up->tzhead.tzh_version[0] == '\0')
-			break;
-		nread -= p - up->buf;
-		memmove(up->buf, p, nread);
+	    }
+
+	    nread -= p - up->buf;
+	    memmove(up->buf, p, nread);
+
+	    /* If this is an old file, we're done.  */
+	    if (!version)
+	      break;
 	}
 	if (doextend && nread > 2 &&
 		up->buf[0] == '\n' && up->buf[nread - 1] == '\n' &&
-- 
2.31.1