FTP: Reject commands with parameters containing CR (#2453)#2455
Open
squidadm wants to merge 1 commit into
Open
FTP: Reject commands with parameters containing CR (#2453)#2455squidadm wants to merge 1 commit into
squidadm wants to merge 1 commit into
Conversation
FTP command syntax treats CR and LF characters as command terminators.
All RFC 959 sightings of CR and LF either refer to the CRLF terminator
or treat both characters the same way. For example, RFC 959 defines an
FTP command parameter as a sequence of chars, where:
<char> ::= any of the 128 ASCII characters except <CR> and <LF>
Squid should (and now does) restrict CR use to the command terminator
sequence, especially since some FTP servers treat CRs as command
delimiters -- if we continue to allow embedded CRs in FTP command
parameters than our FtpClient::writeCommand() will assert when trying to
forward those commands to the FTP server. Moreover, we already use that
CR treatment when _parsing_ FTP responses (see
Ftp::Client::parseControlReply()).
When it comes to command termination, CRs are still optional.
A new ban on CRs in FTP command parameter values means that Squid starts
treating some FTP commands as syntactically invalid, using
EarlyErrorKind::MalformedCommand for the first time since its inception
in 2014 commit eacfca8. For example, `PWD\rQUIT` input is now invalid.
This is a Measurement Factory project.
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
2 participants
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.
FTP command syntax treats CR and LF characters as command terminators.
All RFC 959 sightings of CR and LF either refer to the CRLF terminator
or treat both characters the same way. For example, RFC 959 defines an
FTP command parameter as a sequence of chars, where:
Squid should (and now does) restrict CR use to the command terminator
sequence, especially since some FTP servers treat CRs as command
delimiters -- if we continue to allow embedded CRs in FTP command
parameters than our FtpClient::writeCommand() will assert when trying to
forward those commands to the FTP server. Moreover, we already use that
CR treatment when parsing FTP responses (see
Ftp::Client::parseControlReply()).
When it comes to command termination, CRs are still optional.
A new ban on CRs in FTP command parameter values means that Squid starts
treating some FTP commands as syntactically invalid, using
EarlyErrorKind::MalformedCommand for the first time since its inception
in 2014 commit eacfca8. For example,
PWD\rQUITinput is now invalid.This is a Measurement Factory project.