Skip to content

smtp: reject CR and LF in the envelope address#22119

Closed
alhudz wants to merge 1 commit into
curl:masterfrom
alhudz:smtp-address-reject-crlf
Closed

smtp: reject CR and LF in the envelope address#22119
alhudz wants to merge 1 commit into
curl:masterfrom
alhudz:smtp-address-reject-crlf

Conversation

@alhudz

@alhudz alhudz commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Repro: point --mail-from (or --mail-rcpt) at an address holding a real CR/LF, e.g. sender@example.com followed by %0d%0a then DATA. The server receives the smuggled DATA as its own command line.

Cause: smtp_parse_address() copies the address straight into the MAIL FROM:/RCPT TO: line through Curl_pp_sendf() with no control-byte check. Each SMTP command is one CRLF-terminated line, so an embedded CR or LF in the address splits the line and injects further commands.

Fix: reject a CR or LF in smtp_parse_address(), the shared parser behind MAIL FROM, RCPT TO, AUTH and VRFY/EXPN, before the command is built. Other curl protocol fields built from a single value already refuse control octets; the SMTP envelope addresses did not.

test2110 covers it: before the change the injected line reaches the server, after it curl fails with CURLE_BAD_FUNCTION_ARGUMENT and only EHLO/QUIT go out.

@github-actions github-actions Bot added the tests label Jun 21, 2026
@testclutch

Copy link
Copy Markdown

Analysis of PR #22119 at 989e13f7:

Test 2110 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 120 different CI jobs (the link just goes to one of them).

Generated by Testclutch

@alhudz
alhudz force-pushed the smtp-address-reject-crlf branch from 989e13f to 97fb325 Compare June 21, 2026 16:40
@alhudz

alhudz commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

The test2110 failure was real, thanks. On the Windows/cygwin/msys runners the embedded CR LF got collapsed to a space by the platform argv handling before it ever reached curl, so smtp_parse_address() saw sender@example.com DATA with no control bytes and happily sent it. The lib/smtp.c change is fine; the test just wasn't portable.

Reworked the test to pass the sender through a -K config file (mail-from = "sender@example.com\r\nDATA"), so curl itself decodes the \r\n and the raw CR LF reaches the parser intact on every platform. Local SMTP suite (incl. 2110) is green again.

Comment thread lib/smtp.c Outdated
@bagder

bagder commented Jun 25, 2026

Copy link
Copy Markdown
Member

This branch has conflicts that must be resolved

Can you take a look and force-push?

@alhudz
alhudz force-pushed the smtp-address-reject-crlf branch from b942a3e to d104d89 Compare June 29, 2026 06:20
@alhudz
alhudz marked this pull request as ready for review June 29, 2026 06:20
@alhudz

alhudz commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Rebased on master and force-pushed. The only clash was the test list in tests/data/Makefile.am; test2110 now sits alongside the entries added since. No conflicts left and it shows mergeable. Also flipped it out of draft.

@bagder bagder closed this in bedeeaa Jun 29, 2026
@bagder

bagder commented Jun 29, 2026

Copy link
Copy Markdown
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

3 participants