CVE-2024-27938: Insufficient Information (NVD-CWE-noinfo)
value: Postal is an open source SMTP server. Postal versions less than 3.0.0 are vulnerable to SMTP Smuggling attacks which may allow incoming e-mails to be spoofed. This, in conjunction with a cooperative outgoing SMTP service, would allow for an incoming e-mail to be received by Postal addressed from a server that a user has 'authorised' to send mail on their behalf but were not the genuine author of the e-mail. Postal is not affected for sending outgoing e-mails as email is re-encoded with `<CR><LF>` line endings when transmitted over SMTP. This issue has been addressed and users should upgrade to Postal v3.0.0 or higher. Once upgraded, Postal will only accept End of DATA sequences which are explicitly `<CR><LF>.<CR><LF>`. If a non-compliant sequence is detected it will be logged to the SMTP server log. There are no workarounds for this issue.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-03-11T22:15Z
Repository: https://github.com/postalserver/postal
Language: Ruby
Files changed: app/lib/smtp_server/client.rb, app/lib/smtp_server/server.rb, spec/lib/smtp_server/client/finished_spec.rb
Commit date: 2024-03-01 17:12:10 +0000
Commit message: feat: only accept RFC-compliant End-of-DATA sequence

Previously Postal was fairly forgiving about line endings. This requires that the end of data sequence is `<CR><LF>.<CR><LF>`.

Diff: diff --git a/app/lib/smtp_server/client.rb b/app/lib/smtp_server/client.rb
index 7266aba..53cf678 100644
--- a/app/lib/smtp_server/client.rb
+++ b/app/lib/smtp_server/client.rb
@@ -20,10 +20,14 @@ module SMTPServer
     attr_reader :helo_name
 
     def initialize(ip_address)
       @logging_enabled = true
       @ip_address = ip_address
+
+      @cr_present = false
+      @previous_cr_present = nil
+
       if @ip_address
         check_ip_address
         @state = :welcome
       else
         @state = :preauth
@@ -49,23 +53,32 @@ module SMTPServer
     def id
       @id ||= Nifty::Utils::RandomString.generate(length: 6).upcase
     end
 
     def handle(data)
+      if data[-1] == "\r"
+        @cr_present = true
+        data = data.chop # remove last character (\r)
+      else
+        Postal.logger.debug("\e[33m   WARN: Detected line with invalid line ending (missing <CR>)\e[0m", id: id)
+        @cr_present = false
+      end
+
       Postal.logger.tagged(id: id) do
         if @state == :preauth
           return proxy(data)
         end
 
         log "\e[32m<= #{sanitize_input_for_log(data.strip)}\e[0m"
         if @proc
           @proc.call(data)
-
         else
           handle_command(data)
         end
       end
+    ensure
+      @previous_cr_present = @cr_present
     end
 
     def finished?
       @finished || false
     end
@@ -407,11 +420,11 @@ module SMTPServer
 
       @data << "Received: #{received_header}\r\n"
       @headers["received"] = [received_header]
 
       handler = proc do |idata|
-        if idata == "."
+        if idata == "." && @cr_present && @previous_cr_present
           @logging_enabled = true
           @proc = nil
           finished
         else
           idata = idata.to_s.sub(/\A\.\./, ".")
@@ -422,11 +435,11 @@ module SMTPServer
             log "Not logging further message data."
             @logging_enabled = false
           end
 
           if @receiving_headers
-            if idata.blank?
+            if idata&.length&.zero?
               @receiving_headers = false
             elsif idata.to_s =~ /^\s/
               # This is a continuation of a header
               if @header_key && @headers[@header_key.downcase] && @headers[@header_key.downcase].last
                 @headers[@header_key.downcase].last << idata.to_s
diff --git a/app/lib/smtp_server/server.rb b/app/lib/smtp_server/server.rb
index a08f2a8..dadb48d 100644
--- a/app/lib/smtp_server/server.rb
+++ b/app/lib/smtp_server/server.rb
@@ -192,20 +192,15 @@ module SMTPServer
                 rescue EOFError, Errno::ECONNRESET, Errno::ETIMEDOUT
                   # Client went away
                   eof = true
                 end
 
-                # Normalize all \r\n and \n to \r\n, but ignore only \r.
-                # A \r\n may be split in 2 buffers (\n in one buffer and \r in the other)
-                buffers[io] = buffers[io].gsub(/\r/, "").encode(buffers[io].encoding, crlf_newline: true)
-
                 # We line buffer, so look to see if we have received a newline
                 # and keep doing so until all buffered lines have been processed.
-                while buffers[io].index("\r\n")
+                while buffers[io].index("\n")
                   # Extract the line
-                  line, buffers[io] = buffers[io].split("\r\n", 2)
-
+                  line, buffers[io] = buffers[io].split("\n", 2)
                   # Send the received line to the client object for processing
                   result = client.handle(line)
                   # If the client object returned some data, write it back to the client
                   next if result.nil?
 
diff --git a/spec/lib/smtp_server/client/finished_spec.rb b/spec/lib/smtp_server/client/finished_spec.rb
index e84f4cb..0d8be3a 100644
--- a/spec/lib/smtp_server/client/finished_spec.rb
+++ b/spec/lib/smtp_server/client/finished_spec.rb
@@ -20,16 +20,36 @@ module SMTPServer
       client.handle("MAIL FROM: #{mail_from}")
       client.handle("RCPT TO: #{rcpt_to}")
     end
 
     describe "when finished sending data" do
+      context "when the . character does not end with a <CR>" do
+        it "does nothing" do
+          allow(Postal::Config.smtp_server).to receive(:max_message_size).and_return(1)
+          client.handle("DATA")
+          client.handle("Subject: Hello")
+          client.handle("\r")
+          expect(client.handle(".")).to be nil
+        end
+      end
+
+      context "when the data before the . character does not end with a <CR>" do
+        it "does nothing" do
+          allow(Postal::Config.smtp_server).to receive(:max_message_size).and_return(1)
+          client.handle("DATA")
+          client.handle("Subject: Hello")
+          expect(client.handle(".\r")).to be nil
+        end
+      end
+
       context "when the data is larger than the maximum message size" do
         it "returns an error and resets the state" do
           allow(Postal::Config.smtp_server).to receive(:max_message_size).and_return(1)
           client.handle("DATA")
           client.handle("a" * 1024 * 1024 * 10)
-          expect(client.handle(".")).to eq "552 Message too large (maximum size 1MB)"
+          client.handle("\r")
+          expect(client.handle(".\r")).to eq "552 Message too large (maximum size 1MB)"
         end
       end
 
       context "when a loop is detected" do
         it "returns an error and resets the state" do
@@ -41,11 +61,12 @@ module SMTPServer
           client.handle("Subject: Test")
           client.handle("From: #{mail_from}")
           client.handle("To: #{rcpt_to}")
           client.handle("")
           client.handle("This is a test message")
-          expect(client.handle(".")).to eq "550 Loop detected"
+          client.handle("\r")
+          expect(client.handle(".\r")).to eq "550 Loop detected"
         end
       end
 
       context "when the email content is not suitable for the credential" do
         it "returns an error and resets the state" do
@@ -53,11 +74,12 @@ module SMTPServer
           client.handle("Subject: Test")
           client.handle("From: invalid@krystal.uk")
           client.handle("To: #{rcpt_to}")
           client.handle("")
           client.handle("This is a test message")
-          expect(client.handle(".")).to eq "530 From/Sender name is not valid"
+          client.handle("\r")
+          expect(client.handle(".\r")).to eq "530 From/Sender name is not valid"
         end
       end
 
       context "when sending an outgoing email" do
         let(:domain) { create(:domain, owner: server) }
@@ -69,11 +91,12 @@ module SMTPServer
           client.handle("Subject: Test")
           client.handle("From: #{mail_from}")
           client.handle("To: #{rcpt_to}")
           client.handle("")
           client.handle("This is a test message")
-          expect(client.handle(".")).to eq "250 OK"
+          client.handle("\r")
+          expect(client.handle(".\r")).to eq "250 OK"
           queued_message = QueuedMessage.first
           expect(queued_message).to have_attributes(
             domain: "example.com",
             server: server
           )
@@ -108,11 +131,12 @@ module SMTPServer
             client.handle("Subject: Bounce: Test")
             client.handle("From: #{mail_from}")
             client.handle("To: #{rcpt_to}")
             client.handle("")
             client.handle("This is a test message")
-            expect(client.handle(".")).to eq "250 OK"
+            client.handle("\r")
+            expect(client.handle(".\r")).to eq "250 OK"
 
             queued_message = QueuedMessage.first
             expect(queued_message).to have_attributes(
               domain: Postal::Config.dns.return_path_domain,
               server: server
@@ -139,11 +163,12 @@ module SMTPServer
             client.handle("Subject: Bounce: Test")
             client.handle("From: #{mail_from}")
             client.handle("To: #{rcpt_to}")
             client.handle("")
             client.handle("This is a test message")
-            expect(client.handle(".")).to eq "250 OK"
+            client.handle("\r")
+            expect(client.handle(".\r")).to eq "250 OK"
 
             queued_message = QueuedMessage.first
             expect(queued_message).to have_attributes(
               domain: Postal::Config.dns.return_path_domain,
               server: server
@@ -177,11 +202,12 @@ module SMTPServer
           client.handle("Subject: Test")
           client.handle("From: #{mail_from}")
           client.handle("To: #{rcpt_to}")
           client.handle("")
           client.handle("This is a test message")
-          expect(client.handle(".")).to eq "250 OK"
+          client.handle("\r")
+          expect(client.handle(".\r")).to eq "250 OK"
 
           queued_message = QueuedMessage.first
           expect(queued_message).to have_attributes(
             domain: domain.name,
             server: server

Vulnerable code: 
        if idata == "."
            if idata.blank?
                # Normalize all \r\n and \n to \r\n, but ignore only \r.
                # A \r\n may be split in 2 buffers (\n in one buffer and \r in the other)
                buffers[io] = buffers[io].gsub(/\r/, "").encode(buffers[io].encoding, crlf_newline: true)

                while buffers[io].index("\r\n")
                  line, buffers[io] = buffers[io].split("\r\n", 2)

          expect(client.handle(".")).to eq "552 Message too large (maximum size 1MB)"
          expect(client.handle(".")).to eq "550 Loop detected"
          expect(client.handle(".")).to eq "530 From/Sender name is not valid"
          expect(client.handle(".")).to eq "250 OK"
            expect(client.handle(".")).to eq "250 OK"
            expect(client.handle(".")).to eq "250 OK"
          expect(client.handle(".")).to eq "250 OK"
Fixed code: 
      @cr_present = false
      @previous_cr_present = nil

      if data[-1] == "\r"
        @cr_present = true
        data = data.chop # remove last character (\r)
      else
        Postal.logger.debug("\e[33m   WARN: Detected line with invalid line ending (missing <CR>)\e[0m", id: id)
        @cr_present = false
      end

    ensure
      @previous_cr_present = @cr_present
        if idata == "." && @cr_present && @previous_cr_present
            if idata&.length&.zero?
                while buffers[io].index("\n")
                  line, buffers[io] = buffers[io].split("\n", 2)
      context "when the . character does not end with a <CR>" do
        it "does nothing" do
          allow(Postal::Config.smtp_server).to receive(:max_message_size).and_return(1)
          client.handle("DATA")
          client.handle("Subject: Hello")
          client.handle("\r")
          expect(client.handle(".")).to be nil
        end
      end

      context "when the data before the . character does not end with a <CR>" do
        it "does nothing" do
          allow(Postal::Config.smtp_server).to receive(:max_message_size).and_return(1)
          client.handle("DATA")
          client.handle("Subject: Hello")
          expect(client.handle(".\r")).to be nil
        end
      end

          client.handle("\r")
          expect(client.handle(".\r")).to eq "552 Message too large (maximum size 1MB)"
          client.handle("\r")
          expect(client.handle(".\r")).to eq "550 Loop detected"
          client.handle("\r")
          expect(client.handle(".\r")).to eq "530 From/Sender name is not valid"
          client.handle("\r")
          expect(client.handle(".\r")).to eq "250 OK"
            client.handle("\r")
            expect(client.handle(".\r")).to eq "250 OK"
            client.handle("\r")
            expect(client.handle(".\r")).to eq "250 OK"
          client.handle("\r")
          expect(client.handle(".\r")).to eq "250 OK"