Method "collect_incoming_data" of "SMTPChannel" class should stop buffering if received lines are too long (possible Denial-of-Service attacks).
Without truncating such buffer a simple malicious script sending extremely long lines without "\r\n" terminator could easily saturate system resources.

--- malicious script

import socket
s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
s.connect(("127.0.0.1", 8025))
while 1:
    s.sendall('x' * 1024)

--- proposed smtpd.py patch

124a125

    self.\_\_in_buffer_len = 0

135a137,139

    self.\_\_in_buffer_len += len(data)
    if self.\_\_in_buffer_len \> 4096:
        self.\_\_line = []

Sorry, I realized I've forgotten to reset to zero the bytes counter.
Here's the patch of the patch:

124a125

    self.\_\_in_buffer_len = 0

135a137,140

    self.\_\_in_buffer_len += len(data)
    if self.\_\_in_buffer_len \> 4096:
        self.\_\_line = []
        self.\_\_in_buffer_len = 0

141a147

    self.\_\_in_buffer_len = 0

Patch is inline above.

RFC2822 says lines MUST be less than 998 bytes long, so this should be fine.

What does this do when a line longer than 4096 bytes is found? Does it
report an error to the SMTP client? That's my only concern.

What does this do when a line longer than 4096 bytes
is found? Does it report an error to the SMTP client?
That's my only concern.

Sorry for replying so late.
No, it does not report the error and this is bad.
I've searched through RFCs and I found that RFC 821 and RFC 2821 at
chapter 4.2.2 say that a 500 "Syntax error, command unrecognized"
response could be used to report errors such as command lines too long.

Modified smtpd.py in attachment. It should be definitively fine for
inclusion now.

I update this bug as GvR requested here:
http://groups.google.it/group/python-dev2/browse_thread/thread/33cad7b7c1cdb19f?hl=it
The patch in attachment fixes what discussed before.

In addition it sets a smaller timeout for asyncore.loop() for permitting
to stop the daemon via KeyboardInterrupt immediately.

The patch does not work as Giampaolo intends. If the patch were applied
as-is, no emails longer than 998 bytes could be sent.

Instead, incrementing linelen in the collect_incoming_data() method
should only be performed if self.terminator == '\r\n'.

I can apply a modified version of this patch against trunk after 2.6 is
released. Backports to 2.5 and 2.6 should then be discussed.

Yes, you're right. I mixed up SMTP with FTP which does not send data on
the same connection used for receiving commands.

Given the title, type and severity shouldn't someone take a look at this?

The definite (and only?) solution would be to implement 'Message Size Declaration[1]' Service Extension[2]. We can limit the size of commands and text lines, but not the message size as a whole[3]. RFC1870 was created exactly with the purpose of solving DoS issues[4].

[1] RFC 1870
[2] RFC 1869
[3] RFC 2821, 4.5.3.1
[4] RFC 1870, 9.0

I'm willing to implement a fix (actually, I'm already working on that :-)) --- but I'd make good use of advices since I'm new to this project.

The minimalistic fix to this issue would be to implement Message Size Extension alone and apart from subclasses awareness --- without any changes to the interface, not even to allow changes in the default maximum message size [once SMPTServer is fully-RFC1869-compliant changing these values will be straightforward].

A second (extensive!) approach is to implement a complete ESMTPServer. That's a way bigger task though. It would be wise in this case to implement all the standard extensions, plus support (parsing/error handling) to all RFC1869.

Any thoughts?
My best regards.

NOTE: In my opinion there's no bug, really. All non-extented SMPT servers are vulnerable to resource exhaustion. Maybe we should take this as a feature request not a bug-fix request.

The attached patch adopts the minimalistic approach described in previous post. It pretends to implement Message Size Extension, defining a maximum message data size to 32M bytes and maximum command length to 512 bytes.

In my opinion this is the best way to accomplish a patch to the DoS issue alone. It's still a good idea though to implement full ESMTP support -- I know, I'm repeating myself.

Please note this is my first patch. I'm new to Python and even though I made my best to be in conformance with the "standards" and good practices I may have missed something. Please review this patch with clinical eyes.

For the records: this work is due to Python Bug Day. It worked to attract another curious developer. :-) This project is really awesome. Congratz you all.

My best regards.

Attaching a more concise patch, as requested by georg.brandl.

I think data_size_limit and command_size_limit should be class attributes instead of instance attributes.

Previous patch was incorrect. I'm attaching another one, I'm really sorry.

@giampaolo, about making the limits class attributes, it's not a good idea IMHO. According to RFC1869 command sizes can change depending on which Service Extensions are supported.

size_limits are not class attributes instead of instance attributes, as suggested by giampaolo.rodola.

AFAICT patch looks ok to me.

Committed in r86955. Thanks!