dkim: Fail verification if l= is too large

Currently, if the value of the l= tag is larger than the body, that
causes an out-of-bound access and the server crashes.

The RFC is clear that the value should be <= the size of the
canonicalized body.

This patch fixes the problem by validating the length appropriately, and
returning a permanent failure if it's invalid.

Thanks to John JEAN <john@wargan.com> who reported a similar issue
(fixed in a previous patch), which led to finding this bug.

internal/dkim/verify.go

@@ -190,6 +190,17 @@ func verifySignature(ctx context.Context, sigH header,
 	// https://datatracker.ietf.org/doc/html/rfc6376#section-3.7
 	bodyC := sig.cB.body(body)
 	if sig.l > 0 {
+		// The l= value MUST NOT be larger than the canonicalized body.
+		// https://datatracker.ietf.org/doc/html/rfc6376#section-3.5
+		if sig.l > uint64(len(bodyC)) {
+			trace(ctx, "l= (%d) is larger than the body (%d)",
+				sig.l, len(bodyC))
+			result.Error = fmt.Errorf(
+				"%w: l= (%d) is larger than the body (%d)",
+				errInvalidTag, sig.l, len(bodyC))
+			result.State = PERMFAIL
+			return result
+		}
 		bodyC = bodyC[:sig.l]
 	}
 

internal/dkim/verify_test.go

@@ -2,6 +2,9 @@ package dkim
 
 import (
 	"context"
+	"crypto"
+	"encoding/base64"
+	"errors"
 	"net"
 	"strings"
 	"testing"
@@ -202,6 +205,57 @@ Joe.
 	}
 }
 
+func TestVerifyBodyLength(t *testing.T) {
+	ctx := t.Context()
+	ctx = WithTraceFunc(ctx, t.Logf)
+
+	// Any valid key works, since none of the signatures here are valid. We
+	// use the one from the example in RFC 8463 appendix A.2.
+	ctx = WithLookupTXTFunc(ctx, makeLookupTXT(map[string][]string{
+		"brisbane._domainkey.example.com": {
+			"v=DKIM1; k=ed25519; " +
+				"p=11qYAYKxCrfVS/7TyWQHOg7hcvPapiMlrwIaaPcHURo="},
+	}))
+
+	// Use a valid body hash, so we can tell the l= cases apart: when l= is
+	// the body length, the body hash matches and we fail later on (because
+	// b= is bogus).
+	body := "Hi.\r\n"
+	bh := base64.StdEncoding.EncodeToString(
+		hashWith(crypto.SHA256, []byte(body)))
+
+	cases := []struct {
+		l    string
+		want error
+	}{
+		{"4", ErrBodyHashMismatch},
+		{"5", ErrVerificationFailed},
+		{"6", errInvalidTag},
+		{"18446744073709551615", errInvalidTag},
+	}
+	for _, c := range cases {
+		message := toCRLF(
+			"DKIM-Signature: v=1; a=ed25519-sha256; c=simple/simple;\n"+
+				" d=example.com; s=brisbane; h=from; l="+c.l+";\n"+
+				" bh="+bh+"; b=AAAA\n"+
+				"From: Joe SixPack <joe@example.com>\n"+
+				"\n") + body
+
+		res, err := VerifyMessage(ctx, message)
+		if err != nil {
+			t.Fatalf("l=%s: VerifyMessage returned error: %v", c.l, err)
+		}
+		if res.Found != 1 || res.Valid != 0 {
+			t.Fatalf("l=%s: wanted 1 found, 0 valid; got %v", c.l, res)
+		}
+		r := res.Results[0]
+		if r.State != PERMFAIL || !errors.Is(r.Error, c.want) {
+			t.Errorf("l=%s: wanted PERMFAIL / %v; got %v / %v",
+				c.l, c.want, r.State, r.Error)
+		}
+	}
+}
+
 func TestHeadersToInclude(t *testing.T) {
 	// Test that headersToInclude returns the expected headers.
 	cases := []struct {