Skip to content

Decode percent escapes in FTP user names and passwords - #2091

Open
xroche wants to merge 2 commits into
masterfrom
fz/B18
Open

xroche wants to merge 2 commits into
masterfrom
fz/B18

Conversation

@xroche

@xroche xroche commented Nov 7, 2026

Copy link
Copy Markdown
Owner

An ftp:// URL now percent-decodes its user name and password before USER and PASS, as HTTP Basic and SOCKS5 already do. So ftp://us%40er:p%3Ass@host/ logs in as us@er with password p:ss.

A field that decodes to a control byte is refused, because %0D%0A would inject an FTP command and %00 would cut the name short. A stored password with a literal % must now be written %25.

Test 760 reads the login off the wire of a local FTP server. It fails on master, which sends USER us%40er, and passes here. A mutant that skips the control-byte refusal fails both it and the new -#test=ftp-userpass rows.

xroche and others added 2 commits November 7, 2026 18:36
An ftp:// URL sent its user name and password raw, so
ftp://us%40er:p%3Ass@host/ logged in as "us%40er" while the same
userinfo over HTTP Basic or SOCKS5 means "us@er". Split on the first
raw colon, as SOCKS5 does, then decode each field. A field that decodes
to a control byte is refused: %0D%0A would inject an FTP command, and
%00 would cut the name short. The size bound now applies to the decoded
length.

A stored FTP password holding a literal '%' followed by two hex digits
changes meaning; it must now be written %25.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
The userinfo field is not NUL-terminated at ':' or '@', so this bound
is what keeps an escape inside its field; say so at the guard.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
@xroche
xroche enabled auto-merge (squash) November 7, 2026 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant