Skip to content

Commit

Permalink
dkim: Make timestamp parsing more robust against overflow
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
albertito committed May 10, 2024
1 parent aae0367 commit a1b6821
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 2 deletions.
10 changes: 8 additions & 2 deletions internal/dkim/header.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions internal/dkim/header_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit a1b6821

Please sign in to comment.