Skip to content

ftp: make the MDTM date parser stricter (again)#21041

Closed
bagder wants to merge 2 commits intomasterfrom
bagder/ftp-MDTM
Closed

ftp: make the MDTM date parser stricter (again)#21041
bagder wants to merge 2 commits intomasterfrom
bagder/ftp-MDTM

Conversation

@bagder
Copy link
Copy Markdown
Member

@bagder bagder commented Mar 20, 2026

A previous refactor made the parser more lenient and this takes it back to making sure only ascii digits are accepted.

Added test 1684 to verify

Follow-up to 304b518

Pointed out by Codex Security

A previous refactor made the parser more lenient and this takes it back
to making sure only ascii digits are accepted.

Added test 1684 to verify

Follow-up to 304b518

Pointed out by Codex Security
@bagder bagder added the FTP label Mar 20, 2026
@github-actions github-actions bot added the tests label Mar 20, 2026
@bagder
Copy link
Copy Markdown
Member Author

bagder commented Mar 20, 2026

This is not helpful...

curl/lib/ftp.c:2498:10: error: '*' has higher precedence than '+';
add parentheses to explicitly specify the order  of operations
 [readability-math-missing-parentheses,-warnings-as-errors]
 2498 |   *val = curlx_hexval(p[0]) * 10 + curlx_hexval(p[1]);
      |          ^
      |          (                      )

@bagder bagder marked this pull request as ready for review March 20, 2026 23:50
@bagder bagder requested a review from Copilot March 20, 2026 23:50
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR tightens libcurl’s FTP MDTM (213) timestamp parsing to reject non-ASCII digits (addressing a regression toward leniency) and adds a regression test to ensure malformed MDTM dates no longer affect time-condition behavior.

Changes:

  • Make the FTP 213 date parser require ASCII digits for each 2-digit field (instead of implicitly accepting other characters).
  • Refactor the 2-digit parsing helper to return an error status and write the parsed value via an out-parameter.
  • Add test1684 and register it in the test data Makefile to validate malformed MDTM handling.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
tests/data/test1684 Adds an FTP test case with a malformed MDTM timestamp to ensure stricter parsing and expected transfer behavior under -z.
tests/data/Makefile.am Registers the new test1684 in the test list.
lib/ftp.c Tightens MDTM “213” date parsing by validating digits with ISDIGIT() before converting fields.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@bagder bagder closed this in 322db3e Mar 21, 2026
@bagder bagder deleted the bagder/ftp-MDTM branch March 21, 2026 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants