Do not send FTP commands with embedded CRs or LFs (#2444)#2451
Merged
Conversation
FTP command syntax prohibits bare CRs and LFs except for command termination. FTP servers treat embedded CRs inconsistently. FTP does not have a generally accepted escape mechanism for safely encoding those message-framing characters. Thus, Squid must not send FTP commands with parameters containing embedded CRs or LFs. Squid assembles FTP commands from a variety of information sources that depend, in part, on whether the master transaction originated on an `http(s)_port` or `ftp_port`. One can check for (and reject) embedded CRs and LFs in many code locations. Our attempts to reject invalid inputs earlier, before the assembly, resulted in significant functionality changes going beyond this fix scope but still missed an important injection point. The approach used here covers all known code paths, has a decent chance of remaining effective during significant code refactoring, and does not alter Squid functionality beyond killing transactions that resulted in problematic FTP command parameters (an in-scope change). This approach also allows request adaptation services to "fix" problematic requests in some cases. Rejected transactions usually result in HTTP 502 ERR_FTP_FAILURE responses logged as TCP_MISS_ABORTED. More work is needed to provide better %err_detail for these cases. This is a Measurement Factory project.
kinkie
approved these changes
Jun 27, 2026
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
3 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 prohibits bare CRs and LFs except for command
termination. FTP servers treat embedded CRs inconsistently. FTP does not
have a generally accepted escape mechanism for safely encoding those
message-framing characters. Thus, Squid must not send FTP commands with
parameters containing embedded CRs or LFs.
Squid assembles FTP commands from a variety of information sources that
depend, in part, on whether the master transaction originated on an
http(s)_portorftp_port. One can check for (and reject) embeddedCRs and LFs in many code locations. Our attempts to reject invalid
inputs earlier, before the assembly, resulted in significant
functionality changes going beyond this fix scope but still missed an
important injection point.
The approach used here covers all known code paths, has a decent chance
of remaining effective during significant code refactoring, and does not
alter Squid functionality beyond killing transactions that resulted in
problematic FTP command parameters (an in-scope change). This approach
also allows request adaptation services to "fix" problematic requests in
some cases.
Rejected transactions usually result in HTTP 502 ERR_FTP_FAILURE
responses logged as TCP_MISS_ABORTED. More work is needed to provide
better %err_detail for these cases.
This is a Measurement Factory project.