time: time.Parse accepts out of range value for offsets, and can't always be roundtripped

[arp242]

When passing an offset like +99:99 to time.Parse like so:

func main() {
	t, err := time.Parse("-07:00", "+99:99")
	fmt.Println(err, t.Format(time.RFC3339))
	_, off := t.Zone()
	fmt.Println("offset", off)

	// Round-trip.
	fmt.Println(time.Parse("-07:00", t.Format("-07:00")))
}

The initial Parse will work without error, but the second fails. The above outputs:

<nil> 0000-01-01T00:00:00+100:39
offset 362340
0001-01-01 00:00:00 +0000 UTC parsing time "+100:39" as "-07:00": cannot parse "+100:39" as "-07:00"

The 99 minutes overflows to 01:39. The roundtrip doesn't work because Parse checks if the numbers in hour:min are exactly two digits, so that does return an error.

This is a bit of an obscure edge case that came up during fuzzing; can check that offset >86400 for outrageous offsets to throw an error on parse, but validating the hour:min requires re-parsing the date as that information isn't available.

Everything else (second, minute, hour, day, month) already throws an out of range error so this seems like a good fix. AFAIK these offsets never have and never will be valid. Below is a patch to do exactly that.

commit cfb8f654e8
Author: Martin Tournoij <martin@arp242.net>
Date:   Fri May 17 11:34:12 2024 +0100

    time: return out of range errors for invalid timezone offsets

diff --git a/src/time/format.go b/src/time/format.go
index 875fb36df8..7b43b5bb05 100644
--- a/src/time/format.go
+++ b/src/time/format.go
@@ -1242,6 +1242,12 @@ func parse(layout, value string, defaultLocation, local *Location) (Time, error)
 			if err == nil {
 				ss, _, err = getnum(seconds, true)
 			}
+			if hr > 24 {
+				return Time{}, newParseError(alayout, avalue, "", value, ": offset hour out of range")
+			}
+			if mm > 59 {
+				return Time{}, newParseError(alayout, avalue, "", value, ": offset minute out of range")
+			}
 			zoneOffset = (hr*60+mm)*60 + ss // offset is in seconds
 			switch sign[0] {
 			case '+':
diff --git a/src/time/time_test.go b/src/time/time_test.go
index 86335e3796..d110c28d4c 100644
--- a/src/time/time_test.go
+++ b/src/time/time_test.go
@@ -832,8 +832,8 @@ func TestUnmarshalInvalidTimes(t *testing.T) {
 		{`[]`, "Time.UnmarshalJSON: input is not a JSON string"},
 		{`"2000-01-01T1:12:34Z"`, `<nil>`},
 		{`"2000-01-01T00:00:00,000Z"`, `<nil>`},
-		{`"2000-01-01T00:00:00+24:00"`, `<nil>`},
-		{`"2000-01-01T00:00:00+00:60"`, `<nil>`},
+		{`"2000-01-01T00:00:00+25:00"`, `parsing time "2000-01-01T00:00:00+25:00": offset hour out of range`},
+		{`"2000-01-01T00:00:00+00:60"`, `parsing time "2000-01-01T00:00:00+00:60": offset minute out of range`},
 		{`"2000-01-01T00:00:00+123:45"`, `parsing time "2000-01-01T00:00:00+123:45" as "2006-01-02T15:04:05Z07:00": cannot parse "+123:45" as "Z07:00"`},
 	}
 

With 1.22, and current master branch

[dmitshur]

CC @rsc, @ianlancetaylor.

[seankhliao]

I think this is a dupe of #20555

[ianlancetaylor]

I don't think this is quite #20555. The problem here is not that it can't be roundtripped, it's that we are accepting meaningless values.

[gopherbot]

Change https://go.dev/cl/586717 mentions this issue: time: check for time zone offset overflow