git » chasquid » commit 10aa1a5

diff --git a/internal/dkim/header.go b/internal/dkim/header.go
index 79fb6c4..c942664 100644
--- a/internal/dkim/header.go
+++ b/internal/dkim/header.go
@@ -9,6 +9,8 @@ import (
 	"strconv"
 	"strings"
 	"time"
+
+	"blitiri.com.ar/go/chasquid/internal/normalize"
 )
 
 // https://datatracker.ietf.org/doc/html/rfc6376#section-6
@@ -136,6 +138,20 @@ func (sig *dkimSignature) checkRequiredTags() error {
 		return fmt.Errorf("%w: s=", errMissingRequiredTag)
 	}
 
+	// d= and s= must be valid domain names (a selector has the same syntax).
+	// They are attacker-controlled, and used to build the DNS name of the
+	// key; d= is also included in the Authentication-Results header, so
+	// otherwise it could be used to inject content into it.
+	// https://datatracker.ietf.org/doc/html/rfc6376#section-3.5
+	if _, err := normalize.Domain(sig.d); err != nil {
+		return fmt.Errorf("%w: d= is not a valid domain: %w",
+			errInvalidTag, err)
+	}
+	if _, err := normalize.Domain(sig.s); err != nil {
+		return fmt.Errorf("%w: s= is not a valid selector: %w",
+			errInvalidTag, err)
+	}
+
 	if len(sig.h) > maxHTagEntries {
 		return fmt.Errorf("%w: h= has too many entries (%d > %d)",
 			errInvalidTag, len(sig.h), maxHTagEntries)
diff --git a/internal/dkim/header_test.go b/internal/dkim/header_test.go
index 4b91da8..317a2dc 100644
--- a/internal/dkim/header_test.go
+++ b/internal/dkim/header_test.go
@@ -333,6 +333,30 @@ func TestCheckRequiredTags(t *testing.T) {
 			},
 			err: "invalid tag: h= does not contain 'from'",
 		},
+		{
+			sig: &dkimSignature{
+				a:  "rsa-sha256",
+				b:  []byte("hola que tal"),
+				bh: []byte("como andas"),
+				d:  "example.com dkim=pass header.d=example.net",
+				h:  []string{"from"},
+				s:  "selector",
+			},
+			err: "invalid tag: d= is not a valid domain: " +
+				"invalid domain: invalid character ' '",
+		},
+		{
+			sig: &dkimSignature{
+				a:  "rsa-sha256",
+				b:  []byte("hola que tal"),
+				bh: []byte("como andas"),
+				d:  "example.com",
+				h:  []string{"from"},
+				s:  "sel/ector",
+			},
+			err: "invalid tag: s= is not a valid selector: " +
+				"invalid domain: invalid character '/'",
+		},
 		{
 			sig: &dkimSignature{
 				a:  "rsa-sha256",
@@ -381,6 +405,17 @@ func TestCheckRequiredTags(t *testing.T) {
 			},
 			err: "<nil>",
 		},
+		{
+			sig: &dkimSignature{
+				a:  "rsa-sha256",
+				b:  []byte("hola que tal"),
+				bh: []byte("como andas"),
+				d:  "ñandú.example", // U-labels are allowed (RFC 8616).
+				h:  []string{"from"},
+				s:  "sel_2026.ector", // "_" and "." are allowed.
+			},
+			err: "<nil>",
+		},
 		{
 			sig: &dkimSignature{
 				a:  "rsa-sha256",
diff --git a/internal/dkim/verify_test.go b/internal/dkim/verify_test.go
index 6b92b77..217f578 100644
--- a/internal/dkim/verify_test.go
+++ b/internal/dkim/verify_test.go
@@ -262,6 +262,41 @@ func TestVerifyBodyLength(t *testing.T) {
 	}
 }
 
+func TestVerifyInvalidDomain(t *testing.T) {
+	ctx := t.Context()
+	ctx = WithTraceFunc(ctx, t.Logf)
+	ctx = WithLookupTXTFunc(ctx, makeLookupTXT(map[string][]string{}))
+
+	// A d= tag that is not a valid domain must be rejected, and must not
+	// make it to the Authentication-Results header, where it could inject
+	// content.
+	message := toCRLF(
+		"DKIM-Signature: v=1; a=ed25519-sha256; c=simple/simple;\n" +
+			" d=example.com dkim=pass header.d=example.net; s=sel;\n" +
+			" h=from; bh=AAAA; b=AAAA\n" +
+			"From: Joe SixPack <joe@example.com>\n" +
+			"\n" +
+			"Hi.\n")
+
+	res, err := VerifyMessage(ctx, message)
+	if err != nil {
+		t.Fatalf("VerifyMessage returned error: %v", err)
+	}
+	if res.Found != 1 || res.Valid != 0 {
+		t.Fatalf("wanted 1 found, 0 valid; got %v", res)
+	}
+	r := res.Results[0]
+	if r.State != PERMFAIL || !errors.Is(r.Error, errInvalidTag) {
+		t.Errorf("wanted PERMFAIL / %v; got %v / %v",
+			errInvalidTag, r.State, r.Error)
+	}
+
+	ar := res.AuthenticationResults()
+	if strings.Contains(ar, "example.net") {
+		t.Errorf("Authentication-Results contains the invalid d=: %q", ar)
+	}
+}
+
 func TestHeadersToInclude(t *testing.T) {
 	// Test that headersToInclude returns the expected headers.
 	cases := []struct {