Repository navigation
Conversation
IMAP.sendCommandWithID() concatenates the command ID, command and arguments
into one control line terminated by CRLF, without checking the inputs.
FTP.sendCommand(), POP3.sendCommand(), NNTP.sendCommand() and
SMTP.sendCommand() all reject CR or LF for exactly this reason, so an IMAP
command such as
imap.select("INBOX\r\nA001 DELETE INBOX")
writes two lines to the control connection and executes an attacker-chosen
second command. IMAP.quoteMailboxName() already rejects CR/LF, but that only
covers the arguments that go through it - LOGIN credentials and any direct
sendCommand() call bypass it. The command layer is the last line of defense
and was the only one of the five protocols missing the guard.
The check mirrors the wording and placement of the POP3/NNTP/SMTP guards.
IMAPTest gains four cases covering CR, LF and CRLF in the argument and CR/CRLF
in the command name; they fail with NPE on an unconnected client before this
change and throw IllegalArgumentException with it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
IMAP.sendCommandWithID()concatenates the command ID, command and arguments into one control line terminated by CRLF, without checking the inputs.FTP.sendCommand(),POP3.sendCommand(),NNTP.sendCommand()andSMTP.sendCommand()all reject CR or LF for exactly this reason, so on IMAP a call likewrites two lines to the control connection and executes an attacker-chosen second command. Mailbox names are one realistic carrier (they regularly come from user configuration), and
IMAP.quoteMailboxName()already rejects CR/LF - but that only covers arguments routed through it, whileLOGINcredentials (viaAuthenticatingIMAPClient.login) and any directsendCommand()call bypass it.The command layer is the last line of defense and was the only one of the five protocols missing the guard. The check mirrors the wording and placement of the POP3/NNTP/SMTP guards.
IMAPTestgains four cases covering CR, LF and CRLF in the argument and CR/CRLF in the command name. On an unconnected client they fail withNullPointerExceptionbefore this change and throwIllegalArgumentExceptionwith it.Verification: Windows, JDK 21.
mvn test -Dtest=IMAPTestpasses (19/19 with the fix); reverting onlyIMAP.javafails exactly the four new cases.