pingpong: reject nul byte in server response line

[alhudz]

Repro: point curl at an FTP/IMAP/SMTP/POP3 server that sends a response line with an embedded nul before the LF, e.g. a POP3 greeting +OK ready\u0000injected\r\n. Before this patch curl returns 0 and the line (nul and all) is handed to CURLOPT_HEADERFUNCTION; after it the transfer fails with CURLE_WEIRD_SERVER_REPLY.

Cause: Curl_pp_readresp() finds the line end with memchr on the LF only and passes each line to Curl_client_write(CLIENTWRITE_INFO), which is delivered to the header callback (the response lines are treated as a kind of header). The pingpong path has no nul check, so a malicious or MITM server can smuggle an embedded nul into the header callback. Regular HTTP headers are guarded against this by verify_header(); these control responses are not.

Fix: reject a nul byte in the response line before delivery. verify_header() itself is not reused here as it also enforces a trailing-CR/colon shape that pingpong lines do not have.

[testclutch]

Analysis of PR #21996 at 5350de8e:

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

Generated by Testclutch

[bagder]

How about adding a test case that verifies this as well?

[alhudz]

Added test2108 for the FTP path. That also accounts for the test1282 failure the bot flagged: 1282 still carried the \x00 bytes from the old Kerberos 633-before-gss check, and the new rejection fires before the 633 login-denied path is reached. Dropped those vestigial nuls so 1282 keeps verifying the 633 behaviour, and 2108 covers the nul rejection on its own. Both pass here.

[Copilot]

Pull request overview

This PR hardens the pingpong (FTP/IMAP/POP3/SMTP-style) response parsing path by rejecting server response lines that contain embedded NUL bytes before those lines are forwarded to the client “header” callback.

Changes:

• Add a NUL-byte check in Curl_pp_readresp() and fail the transfer with CURLE_WEIRD_SERVER_REPLY if found.
• Add a new FTP regression test covering NUL injection in a control response line.
• Update an existing FTP test case to avoid embedded NUL bytes now considered invalid.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
lib/pingpong.c	Rejects embedded NUL bytes in pingpong response lines before callback delivery.
tests/data/test2108	New FTP test ensuring NUL bytes in server response lines are rejected.
tests/data/test1282	Removes embedded NUL bytes from an existing FTP server reply to match new validation.
tests/data/Makefile.am	Registers the new test case.

---

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

[bagder]

Thanks!