pythongh-119511: Fix OOM vulnerability in imaplib

serhiy-storchaka · serhiy-storchaka · commit 6322049d1285 · 2024-05-24T21:14:02.000+03:00
The IMAP4 client could consume an arbitrary amount of memory when trying
to connent to a malicious server, because it read a "literal" data with a
single read(size) call, and BufferedReader.read() allocates the bytes
object of the specified size before reading. Now the IMAP4 client reads data
by chunks, therefore the amount of used memory is limited by the
amount of the data actually been sent by the server.
diff --git a/Lib/imaplib.py b/Lib/imaplib.py
@@ -52,6 +52,9 @@
 # search command can be quite large, so we now use 1M.
 _MAXLINE = 1000000
 
+# Data larger than this will be read in chunks, to prevent extreme
+# overallocation.
+_SAFE_BUF_SIZE = 1 << 20
 
 #       Commands
 
@@ -315,6 +318,13 @@ def open(self, host='', port=IMAP4_PORT, timeout=None):
 
     def read(self, size):
         """Read 'size' bytes from remote."""
+        cursize = min(size, _SAFE_BUF_SIZE)
+        data = self.file.read(cursize)
+        while cursize < size and len(data) == cursize:
+            delta = min(cursize, size - cursize)
+            data += self.file.read(delta)
+            cursize += delta
+        return data
         return self.file.read(size)
 
 
diff --git a/Lib/test/test_imaplib.py b/Lib/test/test_imaplib.py
@@ -907,6 +907,18 @@ def handle(self):
             self.assertRaises(imaplib.IMAP4.error,
                               self.imap_class, *server.server_address)
 
+    def test_truncated_large_literal(self):
+        class BadHandler(SimpleIMAPHandler):
+            def handle(self):
+                self._send_textline('* OK {%d}' % size)
+                self._send_textline('IMAP4rev1')
+
+        for w in range(15, 64):
+            size = 1 << w
+            with self.reaped_server(BadHandler) as server:
+                self.assertRaises(imaplib.IMAP4.abort,
+                                  self.imap_class, *server.server_address)
+
     @threading_helper.reap_threads
     def test_simple_with_statement(self):
         # simplest call
diff --git a/Misc/NEWS.d/next/Security/2024-05-24-21-00-52.gh-issue-119511.jKrXQ8.rst b/Misc/NEWS.d/next/Security/2024-05-24-21-00-52.gh-issue-119511.jKrXQ8.rst
@@ -0,0 +1,2 @@
+Fix a vulnerability in the :mod:`imaplib` module, when connecting to a
+malicious server could cause an arbitrary amount of memory to be consumed.

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	+Fix a vulnerability in the :mod:`imaplib` module, when connecting to a
	`2`	`+malicious server could cause an arbitrary amount of memory to be consumed.`