From cb48883ead75d565b5d0298a9c5617835bab0cb0 Mon Sep 17 00:00:00 2001 From: Arne Luenser Date: Wed, 29 Nov 2023 11:35:44 +0100 Subject: [PATCH] fix: reject obviously invalid email addresses from courier --- courier/smtp.go | 4 ++++ courier/smtp_test.go | 22 ++++++++++++++-------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/courier/smtp.go b/courier/smtp.go index 3c60b3361594..c7255da045c7 100644 --- a/courier/smtp.go +++ b/courier/smtp.go @@ -8,6 +8,7 @@ import ( "crypto/tls" "encoding/json" "fmt" + "net/mail" "net/textproto" "strconv" "time" @@ -118,6 +119,9 @@ func (c *courier) QueueEmail(ctx context.Context, t EmailTemplate) (uuid.UUID, e if err != nil { return uuid.Nil, err } + if _, err := mail.ParseAddress(recipient); err != nil { + return uuid.Nil, err + } subject, err := t.EmailSubject(ctx) if err != nil { diff --git a/courier/smtp_test.go b/courier/smtp_test.go index c30d865b88ec..108a9a6f1abf 100644 --- a/courier/smtp_test.go +++ b/courier/smtp_test.go @@ -8,6 +8,7 @@ import ( "crypto/rand" "crypto/rsa" "crypto/tls" + "crypto/x509" "crypto/x509/pkix" "encoding/pem" "flag" @@ -19,8 +20,6 @@ import ( "testing" "time" - "crypto/x509" - "github.com/gofrs/uuid" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -55,21 +54,21 @@ func TestNewSMTP(t *testing.T) { t.SkipNow() } - //Should enforce StartTLS => dialer.StartTLSPolicy = gomail.MandatoryStartTLS and dialer.SSL = false + // Should enforce StartTLS => dialer.StartTLSPolicy = gomail.MandatoryStartTLS and dialer.SSL = false smtp := setupCourier("smtp://foo:bar@my-server:1234/") assert.Equal(t, smtp.SmtpDialer().StartTLSPolicy, gomail.MandatoryStartTLS, "StartTLS not enforced") assert.Equal(t, smtp.SmtpDialer().SSL, false, "Implicit TLS should not be enabled") - //Should enforce TLS => dialer.SSL = true + // Should enforce TLS => dialer.SSL = true smtp = setupCourier("smtps://foo:bar@my-server:1234/") assert.Equal(t, smtp.SmtpDialer().SSL, true, "Implicit TLS should be enabled") - //Should allow cleartext => dialer.StartTLSPolicy = gomail.OpportunisticStartTLS and dialer.SSL = false + // Should allow cleartext => dialer.StartTLSPolicy = gomail.OpportunisticStartTLS and dialer.SSL = false smtp = setupCourier("smtp://foo:bar@my-server:1234/?disable_starttls=true") assert.Equal(t, smtp.SmtpDialer().StartTLSPolicy, gomail.OpportunisticStartTLS, "StartTLS is enforced") assert.Equal(t, smtp.SmtpDialer().SSL, false, "Implicit TLS should not be enabled") - //Test cert based SMTP client auth + // Test cert based SMTP client auth clientCert, clientKey, err := generateTestClientCert() require.NoError(t, err) defer os.Remove(clientCert.Name()) @@ -88,8 +87,8 @@ func TestNewSMTP(t *testing.T) { assert.Equal(t, smtpWithCert.SmtpDialer().TLSConfig.ServerName, "my-server", "TLS config server name should match") assert.Contains(t, smtpWithCert.SmtpDialer().TLSConfig.Certificates, clientPEM, "TLS config should contain client pem") - //error case: invalid client key - conf.Set(ctx, config.ViperKeyCourierSMTPClientKeyPath, clientCert.Name()) //mixup client key and client cert + // error case: invalid client key + conf.Set(ctx, config.ViperKeyCourierSMTPClientKeyPath, clientCert.Name()) // mixup client key and client cert smtpWithCert = setupCourier("smtps://subdomain.my-server:1234/?server_name=my-server") assert.Equal(t, len(smtpWithCert.SmtpDialer().TLSConfig.Certificates), 0, "TLS config certificates should be empty") } @@ -117,6 +116,13 @@ func TestQueueEmail(t *testing.T) { ctx, cancel := context.WithCancel(ctx) defer cancel() + _, err = c.QueueEmail(ctx, templates.NewTestStub(reg, &templates.TestStubModel{ + To: "invalid-email", + Subject: "test-subject-1", + Body: "test-body-1", + })) + require.Error(t, err) + id, err := c.QueueEmail(ctx, templates.NewTestStub(reg, &templates.TestStubModel{ To: "test-recipient-1@example.org", Subject: "test-subject-1",