Skip to content

Commit

Permalink
Merge pull request from GHSA-pr2m-px7j-xg65
Browse files Browse the repository at this point in the history
* SMTP Smuggling Fix

Adapted adherence to RFC 5321 § 2.3.8 to fix SMTP smuggling issues (https://www.rfc-editor.org/rfc/rfc5321#section-2.3.8)

* Apply suggestions from code review

Co-authored-by: Sam Bull <git@sambull.org>

* Add files via upload

* Update test_smtpsmuggling.py

---------

Co-authored-by: Sam Bull <git@sambull.org>
  • Loading branch information
The-Login and Dreamsorcerer committed Mar 2, 2024
1 parent b0267e8 commit 24b6c79
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 5 deletions.
11 changes: 6 additions & 5 deletions aiosmtpd/smtp.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class _DataState(enum.Enum):
EMPTY_BARR = bytearray()
EMPTYBYTES = b''
MISSING = _Missing.MISSING
NEWLINE = '\n'
NEWLINE = '\r\n'
VALID_AUTHMECH = re.compile(r"[A-Z0-9_-]+\Z")

# https://tools.ietf.org/html/rfc3207.html#page-3
Expand Down Expand Up @@ -1427,9 +1427,10 @@ async def smtp_DATA(self, arg: str) -> None:
# Since eof_received cancels this coroutine,
# readuntil() can never raise asyncio.IncompleteReadError.
try:
line: bytes = await self._reader.readuntil()
# https://datatracker.ietf.org/doc/html/rfc5321#section-2.3.8
line: bytes = await self._reader.readuntil(b'\r\n')
log.debug('DATA readline: %s', line)
assert line.endswith(b'\n')
assert line.endswith(b'\r\n')
except asyncio.CancelledError:
# The connection got reset during the DATA command.
log.info('Connection lost during DATA')
Expand All @@ -1446,7 +1447,7 @@ async def smtp_DATA(self, arg: str) -> None:
data *= 0
# Drain the stream anyways
line = await self._reader.read(e.consumed)
assert not line.endswith(b'\n')
assert not line.endswith(b'\r\n')
# A lone dot in a line signals the end of DATA.
if not line_fragments and line == b'.\r\n':
break
Expand All @@ -1458,7 +1459,7 @@ async def smtp_DATA(self, arg: str) -> None:
# Discard data immediately to prevent memory pressure
data *= 0
line_fragments.append(line)
if line.endswith(b'\n'):
if line.endswith(b'\r\n'):
# Record data only if state is "NOMINAL"
if state == _DataState.NOMINAL:
line = EMPTY_BARR.join(line_fragments)
Expand Down
79 changes: 79 additions & 0 deletions aiosmtpd/tests/test_smtpsmuggling.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
# Copyright 2014-2021 The aiosmtpd Developers
# SPDX-License-Identifier: Apache-2.0

"""Test SMTP smuggling."""

from email.mime.text import MIMEText
from smtplib import SMTP, SMTP_SSL
from typing import Generator, Union

import pytest
import smtplib

from aiosmtpd.controller import Controller
from aiosmtpd.testing.helpers import ReceivingHandler
from aiosmtpd.testing.statuscodes import SMTP_STATUS_CODES as S

from aiosmtpd.smtp import SMTP as Server
from aiosmtpd.smtp import Session as ServerSession
from aiosmtpd.smtp import Envelope

from .conftest import Global, controller_data, handler_data

from aiosmtpd.testing.helpers import (
ReceivingHandler
)

def new_data(self, msg):
self.putcmd("data")

(code, repl) = self.getreply()
if self.debuglevel > 0:
self._print_debug('data:', (code, repl))
if code != 354:
raise SMTPDataError(code, repl)
else:
##### Patching input encoding so we can send raw messages
#if isinstance(msg, str):
# msg = smtplib._fix_eols(msg).encode('ascii')
#q = smtplib._quote_periods(msg)
#if q[-2:] != smtplib.bCRLF:
# q = q + smtplib.bCRLF
#q = q + b"." + smtplib.bCRLF
q = msg
self.send(q)
(code, msg) = self.getreply()
if self.debuglevel > 0:
self._print_debug('data:', (code, msg))
return (code, msg)

def return_unchanged(data):
return data

class TestSmuggling:
@handler_data(class_=ReceivingHandler)
def test_smtp_smuggling(self, plain_controller, client):
smtplib._fix_eols = return_unchanged
smtplib._quote_periods = return_unchanged
smtplib.SMTP.data = new_data

handler = plain_controller.handler
sender = "sender@example.com"
recipients = ["rcpt1@example.com"]
resp = client.helo("example.com")
assert resp == S.S250_FQDN
# Trying SMTP smuggling with a fake \n.\r\n end-of-data sequence.
message_data = b"""\
From: Anne Person <anne@example.com>\r\n\
To: Bart Person <bart@example.com>\r\n\
Subject: A test\r\n\
Message-ID: <ant>\r\n\
\r\n\
Testing\
\n.\r\n\
NO SMUGGLING
\r\n.\r\n\
"""
results = client.sendmail(sender, recipients, message_data)
client.quit()
assert b"NO SMUGGLING" in handler.box[0].content

0 comments on commit 24b6c79

Please sign in to comment.