bpo-29606: urllib throwing an exception on any URLs that contain one of '\r\n' for the FTP protocol. by corona10 · Pull Request #1216 · python/cpython

Hi Everyone,

I'm the guy that worked out how to exploit these bugs with firewalls. I just figured I would chime in and provide some opinions.

For one, any URL parsing code that is not FTP-specific should not have input validation checks added. After all, other protocols might be able to handle CR/LF/NUL just fine through an appropriate encoding mechanism. I don't think explicit checks just after decoding are necessarily the best place for this (though I don't see a way to exploit the issue with the proposed patch). In my mind, the fact that CR/LF are an FTP-specific issue and should be dealt with right before FTP commands are dropped on the wire.

For the sake of having more complete test cases, please also consider adding %0d%0a to directory names in your test URLs. E.g.:

'fTp://example.net/foo%0d%0a/file.png',

Finally, I would encourage you to reject any NUL bytes in CWD commands, since these are never valid in any filesystem that I know of (UNIX or Windows).

HTH,
tim