DoS smtpd vulnerability

[giampaolo]

BPO	1745035
Nosy	@warsaw, @birkenfeld, @jcea, @josiahcarlson, @giampaolo
Files	smtpd.py smtpd.diff smtpd.diff: Updated patch (fixes described issue and adds a smaller timeout for asyncore.loop) issue1745035-saviosena-101121.diff issue1745035-101123-saviosena.diff issue1745035-101123-saviosena.diff issue1745035-101123-saviosena.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/josiahcarlson'
closed_at = <Date 2010-12-03.07:38:35.871>
created_at = <Date 2007-06-28.19:44:15.000>
labels = ['type-security', 'library']
title = 'DoS smtpd vulnerability'
updated_at = <Date 2010-12-03.07:38:35.869>
user = 'https://github.com/giampaolo'

bugs.python.org fields:

activity = <Date 2010-12-03.07:38:35.869>
actor = 'georg.brandl'
assignee = 'josiahcarlson'
closed = True
closed_date = <Date 2010-12-03.07:38:35.871>
closer = 'georg.brandl'
components = ['Library (Lib)']
creation = <Date 2007-06-28.19:44:15.000>
creator = 'giampaolo.rodola'
dependencies = []
files = ['8586', '8587', '9272', '19763', '19787', '19788', '19790']
hgrepos = []
issue_num = 1745035
keywords = ['patch']
message_count = 17.0
messages = ['32418', '32419', '32420', '56019', '56627', '61599', '74047', '74073', '116697', '121874', '122064', '122231', '122232', '122235', '122239', '122241', '123195']
nosy_count = 9.0
nosy_names = ['barry', 'georg.brandl', 'jafo', 'jcea', 'josiahcarlson', 'giampaolo.rodola', 'BreamoreBoy', 'henriquebastos', 'saviosena']
pr_nums = []
priority = 'high'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'security'
url = 'https://bugs.python.org/issue1745035'
versions = ['Python 2.6', 'Python 2.5', 'Python 3.1', 'Python 2.7', 'Python 3.2']

[giampaolo]

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.

[giampaolo]

--- 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 = []

[giampaolo]

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

[jafo]

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.

[giampaolo]

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.

[giampaolo]

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.

[josiahcarlson]

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.