Skip to content

Improve parsing of certain FTP directory listing formats#2408

Closed
rousskov wants to merge 1 commit intosquid-cache:masterfrom
measurement-factory:SQUID-111-ftp-listing-wo-name
Closed

Improve parsing of certain FTP directory listing formats#2408
rousskov wants to merge 1 commit intosquid-cache:masterfrom
measurement-factory:SQUID-111-ftp-listing-wo-name

Conversation

@rousskov
Copy link
Copy Markdown
Contributor

This surgical fix restricts parsing to the input buffer when the listing
entry date in "TypeA" or "TypeB" formats is not followed by a filename.
It does not improve rendering of listings with missing filenames or the
overall quality of FTP listing parsing code.

C strchr() always returns a non-nil pointer when given a NUL character,
so its callers must be careful not to supply a NUL character if a
"natural" one-of-the-regular-c-string-characters membership test is
required.

The bug was probably introduced in 1997 commit 3fdadc7 and then
duplicated in 2017 commit 3d87209.

This surgical fix restricts parsing to the input buffer when the listing
entry date in "TypeA" or "TypeB" formats is not followed by a filename.
It does not improve rendering of listings with missing filenames or the
overall quality of FTP listing parsing code.

C strchr() always returns a non-nil pointer when given a NUL character,
so its callers must be careful not to supply a NUL character if a
"natural" one-of-the-regular-c-string-characters membership test is
required. The bug was probably introduced in 1997 commit 3fdadc7 and
then duplicated in 2017 commit 3d87209.
@rousskov
Copy link
Copy Markdown
Contributor Author

I have developed this fix independently, but one of the two patches quoted in the original bug report has the same correct fix (the other one should not be used).

AFAICT, there are other places in Squid code where strchr() is used unsafely or dangerously, but I think it is best to keep this PR dedicated to this ftpListParseParts() fix.

@rousskov rousskov added the S-could-use-an-approval An approval may speed this PR merger (but is not required) label Apr 17, 2026
@kinkie
Copy link
Copy Markdown
Contributor

kinkie commented Apr 18, 2026

LGTM

@kinkie kinkie added M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels backport-to-v7 maintainer has approved these changes for v7 backporting labels Apr 18, 2026
squid-anubis pushed a commit that referenced this pull request Apr 19, 2026
This surgical fix restricts parsing to the input buffer when the listing
entry date in "TypeA" or "TypeB" formats is not followed by a filename.
It does not improve rendering of listings with missing filenames or the
overall quality of FTP listing parsing code.

C strchr() always returns a non-nil pointer when given a NUL character,
so its callers must be careful not to supply a NUL character if a
"natural" one-of-the-regular-c-string-characters membership test is
required.

The bug was probably introduced in 1997 commit 3fdadc7 and then
duplicated in 2017 commit 3d87209.
@squid-anubis squid-anubis added the M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels label Apr 19, 2026
@squid-anubis squid-anubis added M-merged https://github.com/measurement-factory/anubis#pull-request-labels and removed M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels labels Apr 19, 2026
squidadm pushed a commit to squidadm/squid that referenced this pull request Apr 20, 2026
…#2408)

This surgical fix restricts parsing to the input buffer when the listing
entry date in "TypeA" or "TypeB" formats is not followed by a filename.
It does not improve rendering of listings with missing filenames or the
overall quality of FTP listing parsing code.

C strchr() always returns a non-nil pointer when given a NUL character,
so its callers must be careful not to supply a NUL character if a
"natural" one-of-the-regular-c-string-characters membership test is
required.

The bug was probably introduced in 1997 commit 3fdadc7 and then
duplicated in 2017 commit 3d87209.
@squidadm squidadm removed the backport-to-v7 maintainer has approved these changes for v7 backporting label Apr 20, 2026
@squidadm
Copy link
Copy Markdown
Collaborator

queued for backport to v7

@rousskov rousskov removed the S-could-use-an-approval An approval may speed this PR merger (but is not required) label Apr 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

M-merged https://github.com/measurement-factory/anubis#pull-request-labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants