This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author ecbftw
Recipients Nam.Nguyen, ecbftw, martin.panter, serhiy.storchaka, vstinner
Date 2017-07-22.14:46:05
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1500734766.84.0.294113156957.issue30713@psf.upfronthosting.co.za>
In-reply-to
Content
I'm the guy that did the original security research on this issue.  I've been a pentester for over 12 years, where I am regularly helping developers understand how to best correct their injection flaws. Please carefully consider what I'm trying to tell you. I've been trying to get this message through to Python for nearly 2 years now. =(


Serhiy has the right idea.  His email here shows a firm understanding of the issue and the correct remediation approach:
  https://mail.python.org/pipermail/python-dev/2017-July/148715.html

It seems every developer initially assumes input validation is the way you deal with injection flaws. But this creates a false sense of a "trade off" between security and usability. Rejecting evil-looking things up front just creates more problems for you. Also, you can't be sure every code path leading from user input to the injection point is addressed. (What other ways can new lines reach the FTP library?)

Instead, the problem needs to be dealt with in the lines of code just before the injection occurs. Ideally, any special character should be encoded/escaped, in which case you get the best of both worlds: security and full usability. However, when you're dealing with a syntax that was poorly designed and doesn't have a standard way to escape special characters, then you are forced to reject them. Still, it is better to do this rejection right before the commands are dropped onto the TCP stream. That way, if you later decide that there's an acceptable way to encode new lines for *specific* commands, then you can perform that encoding right there and relax the restrictions in specific cases.

I like Serhiy's idea of optionally supporting escaping via the syntax defined in RFC 2640. This should be disabled by default, and a short-term security patch shouldn't try to tackle it. But in a new Python release, a switch could be added that causes new lines to be escaped for specific commands, such as the CWD and GET commands.  It is likely that this escaping is only going to be useful in specific commands, based on what FTP servers actually support. Of course none of this is possible if you use a heavy-handed approach of blocking %0a in URLs up front.
History
Date User Action Args
2017-07-22 14:46:06ecbftwsetrecipients: + ecbftw, vstinner, martin.panter, Nam.Nguyen, serhiy.storchaka
2017-07-22 14:46:06ecbftwsetmessageid: <1500734766.84.0.294113156957.issue30713@psf.upfronthosting.co.za>
2017-07-22 14:46:06ecbftwlinkissue30713 messages
2017-07-22 14:46:05ecbftwcreate