From f3374e7923543fedcf1487751b50efa727c9fd76 Mon Sep 17 00:00:00 2001 From: Alberto Bertogli Date: Mon, 12 Oct 2026 20:16:38 +0100 Subject: [PATCH] smtpsrv: Always sanitize HELO/EHLO domain Today we use the HELO/EHLO domain in a variety of places, which are informational, but some have specific syntax (like the Received header). Using the un-sanitized values can lead to some confusing and/or invalid values. This is not very risky today, but it could be problematic in the future if these strings are used in other contexts. To prevent those problems, this patch sanitizes the HELO/EHLO domain when receiving it. Hooks used to receive the sanitized and raw variants; now, both are sanitized (the raw one is kept for backwards compatibility). If the sanitization leaves the string empty, we use the hard-coded no-valid-characters.invalid as a placeholder, to make this situation easier to identify when troubleshooting. --- docs/hooks.md | 7 ++++--- internal/smtpsrv/conn.go | 28 +++++++++++++++++++++++----- internal/smtpsrv/conn_test.go | 26 ++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 8 deletions(-) diff --git a/docs/hooks.md b/docs/hooks.md index 97fb127..bf8a7d1 100644 --- a/docs/hooks.md +++ b/docs/hooks.md @@ -42,9 +42,10 @@ The environment will contain the following variables: - `$PWD`: The working directory, which will be the config directory. - `$REMOTE_ADDR`: IP address of the remote side of the connection. - `$EHLO_DOMAIN`: EHLO/HELO domain, as given by the client; sanitized for - safety. - - `$EHLO_DOMAIN_RAW`: Same as `$EHLO_DOMAIN`, but not sanitized; be careful as - it can contain problematic characters. + safety (only letters, digits, and `-.[]:` are kept). If there are no valid + characters, it is `no-valid-characters.invalid`. + - `$EHLO_DOMAIN_RAW`: Same as `$EHLO_DOMAIN`. Kept for backwards + compatibility; it used to be the unsanitized value. - `$MAIL_FROM`: MAIL FROM address. - `$RCPT_TO`: RCPT TO addresses, space separated. - `$AUTH_AS`: Authenticated user; empty if the connection has not diff --git a/internal/smtpsrv/conn.go b/internal/smtpsrv/conn.go index c4b1934..ba01e0e 100644 --- a/internal/smtpsrv/conn.go +++ b/internal/smtpsrv/conn.go @@ -356,7 +356,7 @@ func (c *Conn) HELO(params string) (code int, msg string) { if len(strings.TrimSpace(params)) == 0 { return 501, "Invisible customers are not welcome!" } - c.ehloDomain = strings.Fields(params)[0] + c.setEHLODomain(params) types := []string{ "general store", "used armor dealership", "second-hand bookstore", @@ -374,7 +374,7 @@ func (c *Conn) EHLO(params string) (code int, msg string) { if len(strings.TrimSpace(params)) == 0 { return 501, "Invisible customers are not welcome!" } - c.ehloDomain = strings.Fields(params)[0] + c.setEHLODomain(params) c.isESMTP = true buf := bytes.NewBuffer(nil) @@ -871,11 +871,26 @@ func checkData(data []byte) error { return nil } +// Placeholder EHLO domain, used when the one given by the client has no valid +// characters. The .invalid TLD is reserved for names that are guaranteed to +// be invalid (RFC 6761). +const invalidEHLODomain = "no-valid-characters.invalid" + +// setEHLODomain sets the EHLO domain from the (non-empty) HELO/EHLO +// parameters, sanitizing it. +func (c *Conn) setEHLODomain(params string) { + c.ehloDomain = sanitizeEHLODomain(strings.Fields(params)[0]) + if c.ehloDomain == "" { + c.ehloDomain = invalidEHLODomain + } +} + // Sanitize HELO/EHLO domain. // RFC is extremely flexible with EHLO domain values, allowing all printable // ASCII characters. They can be tricky to use in shell scripts (commonly used -// as post-data hooks), so this function sanitizes the value to make it -// shell-safe. +// as post-data hooks), and in headers (e.g. "(", ")" and ";" have special +// meanings in the Received header), so this function sanitizes the value to +// make it safe for both. func sanitizeEHLODomain(s string) string { var n strings.Builder for _, c := range s { @@ -918,7 +933,10 @@ func (c *Conn) runPostDataHook(data []byte) ([]byte, bool, error) { cmd.Env = append(cmd.Env, v+"="+os.Getenv(v)) } cmd.Env = append(cmd.Env, "REMOTE_ADDR="+c.remoteAddr.String()) - cmd.Env = append(cmd.Env, "EHLO_DOMAIN="+sanitizeEHLODomain(c.ehloDomain)) + cmd.Env = append(cmd.Env, "EHLO_DOMAIN="+c.ehloDomain) + + // The EHLO domain is already sanitized, so this is the same as + // EHLO_DOMAIN. We keep it for backwards compatibility. cmd.Env = append(cmd.Env, "EHLO_DOMAIN_RAW="+c.ehloDomain) cmd.Env = append(cmd.Env, "MAIL_FROM="+c.mailFrom) cmd.Env = append(cmd.Env, "RCPT_TO="+strings.Join(c.rcptTo, " ")) diff --git a/internal/smtpsrv/conn_test.go b/internal/smtpsrv/conn_test.go index fad3bb1..75e538a 100644 --- a/internal/smtpsrv/conn_test.go +++ b/internal/smtpsrv/conn_test.go @@ -142,3 +142,29 @@ func TestSanitizeEHLODomain(t *testing.T) { } } } + +func TestEHLODomainIsSanitized(t *testing.T) { + cases := []struct { + params string + expected string + }{ + {"mx.example.com", "mx.example.com"}, + {"[1.2.3.4] extra params", "[1.2.3.4]"}, + {"evil;(x", "evilx"}, + {"!!!", invalidEHLODomain}, + } + for _, c := range cases { + for _, cmd := range []string{"HELO", "EHLO"} { + conn := &Conn{} + if cmd == "HELO" { + conn.HELO(c.params) + } else { + conn.EHLO(c.params) + } + if conn.ehloDomain != c.expected { + t.Errorf("%s %q: got EHLO domain %q, expected %q", + cmd, c.params, conn.ehloDomain, c.expected) + } + } + } +}