From a1b6821ce12acbaed40c902763b715cf7f691666 Mon Sep 17 00:00:00 2001
From: Alberto Bertogli <albertito@blitiri.com.ar>
Date: Fri, 10 May 2024 16:47:22 +0100
Subject: [PATCH] dkim: Make timestamp parsing more robust against overflow

The timestamp string in the t= and x= headers is an "unsigned decimal
integer", but time.Unix takes an int64. Today we parse it as uint64 and
then cast it, but this can cause issues with overflow and type
conversion.

This patch fixes that by parsing the timestamps as signed integers, and
then checking they're positive.
---
 internal/dkim/header.go      | 10 ++++++++--
 internal/dkim/header_test.go | 12 ++++++++++++
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/internal/dkim/header.go b/internal/dkim/header.go
index ece056d..da3a6a6 100644
--- a/internal/dkim/header.go
+++ b/internal/dkim/header.go
@@ -144,6 +144,7 @@ var (
 	errUnsupportedHash    = errors.New("unsupported hash")
 	errUnsupportedKeyType = errors.New("unsupported key type")
 	errMissingRequiredTag = errors.New("missing required tag")
+	errNegativeTimestamp  = errors.New("negative timestamp")
 )
 
 // String replacer that removes whitespace.
@@ -257,11 +258,16 @@ func dkimSignatureFromHeader(header string) (*dkimSignature, error) {
 }
 
 func unixStrToTime(s string) (time.Time, error) {
-	ti, err := strconv.ParseUint(s, 10, 64)
+	// Technically the timestamp is an "unsigned decimal integer", but since
+	// time.Unix takes an int64, we use that and check it's positive.
+	ti, err := strconv.ParseInt(s, 10, 64)
 	if err != nil {
 		return time.Time{}, err
 	}
-	return time.Unix(int64(ti), 0), nil
+	if ti < 0 {
+		return time.Time{}, errNegativeTimestamp
+	}
+	return time.Unix(ti, 0), nil
 }
 
 type keyType string
diff --git a/internal/dkim/header_test.go b/internal/dkim/header_test.go
index 5902bb1..1ebfbdf 100644
--- a/internal/dkim/header_test.go
+++ b/internal/dkim/header_test.go
@@ -146,6 +146,18 @@ func TestSignatureFromHeader(t *testing.T) {
 			want: nil,
 			err:  strconv.ErrSyntax,
 		},
+		{
+			// Invalid t= tag.
+			in:   "v=1; a=rsa-sha256; t=-12345",
+			want: nil,
+			err:  errNegativeTimestamp,
+		},
+		{
+			// Invalid x= tag.
+			in:   "v=1; a=rsa-sha256; x=-1234",
+			want: nil,
+			err:  errNegativeTimestamp,
+		},
 		{
 			// Unknown hash algorithm.
 			in:   "v=1; a=rsa-sxa666",