From 4877066af2fba3b85da8a144e3362b9df34dcaec Mon Sep 17 00:00:00 2001 From: Mathias Lieber Date: Thu, 4 Jan 2024 09:28:32 +0100 Subject: [PATCH] Prevent . SMTP smuggling attacks --- data.go | 5 +--- server_test.go | 78 ++++++++++++++++++++++++++++++++++---------------- 2 files changed, 55 insertions(+), 28 deletions(-) diff --git a/data.go b/data.go index d2d3d8f..4456774 100644 --- a/data.go +++ b/data.go @@ -82,7 +82,7 @@ func (r *dataReader) Read(b []byte) (n int, err error) { // not rewrite CRLF -> LF. // Run data through a simple state machine to - // elide leading dots and detect ending .\r\n line. + // elide leading dots and detect End-of-Data (.) line. const ( stateBeginLine = iota // beginning of line; initial state; must be zero stateDot // read . at beginning of line @@ -129,9 +129,6 @@ func (r *dataReader) Read(b []byte) (n int, err error) { if c == '\r' { r.state = stateCR } - if c == '\n' { - r.state = stateBeginLine - } } b[n] = c n++ diff --git a/server_test.go b/server_test.go index 759385b..cc6543b 100644 --- a/server_test.go +++ b/server_test.go @@ -735,32 +735,62 @@ func TestServer_tooLongMessage(t *testing.T) { // See https://www.postfix.org/smtp-smuggling.html func TestServer_smtpSmuggling(t *testing.T) { - be, s, c, scanner := testServerAuthenticated(t) - defer s.Close() - - io.WriteString(c, "MAIL FROM:\r\n") - scanner.Scan() - io.WriteString(c, "RCPT TO:\r\n") - scanner.Scan() - io.WriteString(c, "DATA\r\n") - scanner.Scan() - - io.WriteString(c, "This is a message with an SMTP smuggling dot:\r\n") - io.WriteString(c, ".\n") - io.WriteString(c, "Final dot comes after.\r\n") - io.WriteString(c, ".\r\n") - scanner.Scan() - if !strings.HasPrefix(scanner.Text(), "250 ") { - t.Fatal("Invalid DATA response, expected an error but got:", scanner.Text()) - } + cases := []struct { + name string + lines []string + expected string + }{ + { + name: ".", + lines: []string{ + "This is a message with an SMTP smuggling dot:\r\n", + ".\n", + "Final dot comes after.\r\n", + ".\r\n", + }, + expected: "This is a message with an SMTP smuggling dot:\r\n\nFinal dot comes after.\r\n", + }, + { + name: ".", + lines: []string{ + "This is a message with an SMTP smuggling dot:\n", // not a line on its own + ".\r\n", + "Final dot comes after.\r\n", + ".\r\n", + }, + expected: "This is a message with an SMTP smuggling dot:\n.\r\nFinal dot comes after.\r\n", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + be, s, c, scanner := testServerAuthenticated(t) + defer s.Close() + + io.WriteString(c, "MAIL FROM:\r\n") + scanner.Scan() + io.WriteString(c, "RCPT TO:\r\n") + scanner.Scan() + io.WriteString(c, "DATA\r\n") + scanner.Scan() + + for _, line := range tc.lines { + io.WriteString(c, line) + } + scanner.Scan() + if !strings.HasPrefix(scanner.Text(), "250 ") { + t.Fatal("Invalid DATA response, expected an error but got:", scanner.Text()) + } - if len(be.messages) != 1 { - t.Fatal("Invalid number of sent messages:", len(be.messages)) - } + if len(be.messages) != 1 { + t.Fatal("Invalid number of sent messages:", len(be.messages)) + } - msg := be.messages[0] - if string(msg.Data) != "This is a message with an SMTP smuggling dot:\r\n\nFinal dot comes after.\r\n" { - t.Fatalf("Invalid mail data: %q", string(msg.Data)) + msg := be.messages[0] + if string(msg.Data) != tc.expected { + t.Fatalf("Invalid mail data: %q", string(msg.Data)) + } + }) } }