Hi Stas, On Mon, Feb 23, 2015 at 7:00 AM, Stanislav Malyshev <smalys...@gmail.com> wrote:
> > I think this will be the final discussion before vote. > > This RFC is to make PHP stronger against script inclusion attacks just > like > > other languages. > > > > https://wiki.php.net/rfc/script_only_include > > I still think this RFC takes a wrong road for the following reasons: > > 1. Having any code in your app that allows to run include on > user-controlled files (I'm not talking about filtered cases but user > data controlling the path) is insecure and can not be made secure. It > should just never be done. Trying to find workarounds for this is like > safe_mode - good idea in theory, leads to worse security in practice. > This is mitigation proposal against script inclusions. The difference is clear by statistics. Because this is mitigation, it does not aims to be a perfect solution. It aims to make PHP as secure as other languages. I think system admins feel more comfortable with this change, too. They know PHP programs are very weak against script inclusion attacks compare to other languages. > > 2. Default configuration would break tons of PHP scripts with extensions > other than .php (very frequent case). The BC break potential of this is > very big as it modifies core functionality. > Compatibility can be provided by one liner. ini_set('zend.script_extensions', '.php .phar .inc .phtml .php4 .php5'); ini_set() does not emit any errors for non existing INIs. 3. Prohibiting phar uploads would also be a bc break, but more > importantly, there still probably are ways to work around this by using > phar files with extension different than .phar and then asking to > include files within that phar file. As long as the eventual path would > end in .php, your code would allow it. > Security is trade off relation, so I think this change acceptable trade off to disable "script inclusion" (executing attacker programs). Users can move uploaded files safely without move_uploaded_file() now. I just made use of it to provide another mitigation, since script only include cannot be mitigation for uploading script files under docroot. Also, the claim that move_upload_file() is obsolete is not based on > anything as far as I can see. Why is it "obsolete"? move_uplaoded_file() is needed for "register_globals". Attacker could specify source files (i.e. in $_FILES) other than uploaded files with "register_globals". Current move_uploaded_file() checks source filename is really a uploaded file's filename. It prevents moving other files, so it's not completely useless but there is not real protections now because values in $_FILES is safe now. I know your point of view, but I hope you like this RFC. Thank you for your comment. Your comments are very helpful to come up with this RFC. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
