smtplib: unlimited readline() from connection

[tiran]

BPO	16042
Nosy	@warsaw, @akuchling, @birkenfeld, @doko42, @larryhastings, @giampaolo, @tiran, @benjaminp, @bitdancer, @serhiy-storchaka
Files	smtp_readline.patch smtplib-2.6.patch smtplib-2.6-with-test.patch smtplib_maxline-2.7.patch 3.3-fix.txt

^{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/tiran'
closed_at = <Date 2014-12-06.01:17:19.974>
created_at = <Date 2012-09-25.10:40:09.346>
labels = ['release-blocker', 'library', 'expert-email', 'performance']
title = 'smtplib: unlimited readline() from connection'
updated_at = <Date 2014-12-06.01:17:19.973>
user = 'https://github.com/tiran'

bugs.python.org fields:

activity = <Date 2014-12-06.01:17:19.973>
actor = 'python-dev'
assignee = 'christian.heimes'
closed = True
closed_date = <Date 2014-12-06.01:17:19.974>
closer = 'python-dev'
components = ['Library (Lib)', 'email']
creation = <Date 2012-09-25.10:40:09.346>
creator = 'christian.heimes'
dependencies = []
files = ['27297', '31772', '31773', '32254', '32590']
hgrepos = []
issue_num = 16042
keywords = ['patch']
message_count = 38.0
messages = ['171245', '171294', '171296', '171297', '171298', '171301', '171303', '173413', '180270', '180295', '180306', '182200', '182201', '182202', '182256', '185058', '196858', '197779', '197782', '197786', '197789', '197792', '197794', '197796', '197799', '197800', '197809', '198302', '200347', '200591', '202722', '210866', '211694', '211697', '214909', '227892', '227932', '232232']
nosy_count = 12.0
nosy_names = ['barry', 'akuchling', 'georg.brandl', 'doko', 'larry', 'giampaolo.rodola', 'christian.heimes', 'benjamin.peterson', 'Arfrever', 'r.david.murray', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'release blocker'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue16042'
versions = ['Python 2.7']

[tiran]

This bug is similar to bpo-16037 and a modified copy of bpo-16038.

The smtplib module doesn't limit the amount of read data in its call to readline(). An erroneous or malicious SMTP server can trick the smtplib module to consume large amounts of memory.

Suggestion:
The smtplib module should be modified to use limited readline() with _MAXLINE like the httplib module.

[tiran]

RFC 821 [1] specifies rather short line lengths between 512 and 1001 chars including the trailing CRLF. A line limit of a couple of kilobyte should definitely work with all standard conform SMTP clients and servers.

[1] http://www.freesoft.org/CIE/RFC/821/24.htm

[tiran]

First patch

I haven't written tests yet nor implemented the size limit on the mock_socket class.

[bitdancer]

I've only taken a quick glance at this so far.

Why size=-1 instead of size=None?

[tiran]

size=-1 mimics the code of the io module. The C implementation of readline() maps all negative values to "unlimited" and values >= 0 as limit.

>>> sys.stdin.readline(None)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'NoneType' object cannot be interpreted as an integer

[serhiy-storchaka]

Both io.IOBase.readline() and io.TextIOBase.readline() have parameter named "limit".

I doubt that such a change can be done in 2.7 and 3.2.

[serhiy-storchaka]

Please submit patches in standard Mercurial format to them understood Rietveld. I wanted to make a code review, but I don't see the definition of readline() method in the file Lib/smtplib.py.

[serhiy-storchaka]

I understand you, it's a patch against 2.7.

•    def readline(self, size=-1):
  

In io.IOBase.readline() and in io.TextIOBase.readline() this parameter named "limit".

•            if size is not None and len(str) == size:
  

•                break
  

It can be moved to the while condition:

             while chr != b"\n" and (size is None or len(str) < size):

[bitdancer]

Christian, do you want to try to complete this before the 2.7.4 RC?

[tiran]

Yes, I'm going to work on this issue for 2.7 and 3.3.

[serhiy-storchaka]

mock_socket violates readline() contract. It can return more than size bytes, and this can break SMTP.readline().