Skip to content

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

@arp242

Description

@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

Metadata

Metadata

Assignees

No one assigned

    Labels

    FixPendingIssues that have a fix which has not yet been reviewed or submitted.FrozenDueToAgeNeedsFixThe path to resolution is known, but the work has not been done.

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions