Skip to content

gh-160369: Reject buffered plaintext after IMAP STARTTLS - #160370

Open
JelleZijlstra wants to merge 5 commits into
python:mainfrom
JelleZijlstra:codex/imap-starttls
Open

JelleZijlstra wants to merge 5 commits into
python:mainfrom
JelleZijlstra:codex/imap-starttls

Conversation

@JelleZijlstra

@JelleZijlstra JelleZijlstra commented Oct 22, 2026 •

Copy link
Copy Markdown
Member

Reject unexpected plaintext buffered after the STARTTLS response before upgrading the connection, so it cannot be interpreted as TLS-protected server responses. Add regression coverage and a Security NEWS entry.

Closes #160369.

@bedevere-app bedevere-app Bot mentioned this pull request Oct 22, 2026
@JelleZijlstra JelleZijlstra added the type-security A security issue label Oct 22, 2026 — with ChatGPT Codex Connector
@JelleZijlstra
JelleZijlstra marked this pull request as ready for review October 22, 2026 22:01
@JelleZijlstra
JelleZijlstra requested a review from a team as a code owner October 22, 2026 22:01
Comment thread Lib/imaplib.py
if typ == 'OK':
if any(self._readbuf):
raise self.abort('unexpected data after STARTTLS response')
self._readbuf = []

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to discard all cached state that could have been received before a STARTTLS command, so that includes untagged_responses, too. We should add a test for this case.

@savannahostrowski

Copy link
Copy Markdown
Member

@JelleZijlstra @sethmlarson I was thinking it'd be good to get this into alpha 2 tomorrow if we can. Do you folks think that's doable?

@sethmlarson

sethmlarson commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

@savannahostrowski I think so, @JelleZijlstra I can take over submitting a fix for my comment?

@sethmlarson

Copy link
Copy Markdown
Contributor

@JelleZijlstra Pushed a fix here for you to review: 55f6dda

Comment thread Lib/test/test_imaplib.py Outdated
ssl_context=ssl_context)
client.shutdown()

def test_starttls_discards_untagged_responses(self):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test should be moved into NewIMAPTests, the SSL fixture already makes all commands run as TLS so we don't test what we need to test.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the test case in cfc62f0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

Pending bug

4 participants