Re: PHP File Upload Security Hole - Still No Fix?

At 13:44 05/09/2000, Jon Ribbens wrote:

WARNING: Sarcasm and useful content both feature below. Kudos to readers

         who manage to extract the one from the other.

I've just been reading in BUGTRAQ about the PHP File Upload security hole. Rasmus' 'fix' was as excellent as can be expected of him. However, I am somewhat mystified as to in what way Zeev's 'fix' is in any way a fix for the problem. Just like Rasmus, he appears to have completely misunderstood what the problem is, and applied a fix that bears no relevance to it whatsoever. Zeev's patch only affects the file 'rfc1867.c'. Why? This file is not where the problem is. There is no change that can be made to this file to fix it. The code in this file is only called if the browser makes a POST request of type multipart/form-data. So, uh, the evil hacker doesn't do that. They use a normal POST or a GET request instead. Or, they use a multipart/form-data request *but don't send a file*! Gah! What deviousness! Despite what Rasmus said on BUGTRAQ, the original poster was quite correct in saying that this is a problem caused by register_globals. The original poster simply underestimated how broken PHP is. Even if you do not use 'register_globals', there is still no way to tell the difference between a genuine PHP-set filename variable, and an evil hacker-set one.

I'm not sure why you think you earned the right to use that condescending tone, but regardless. That's not true. It is common knowledge, that anything that is submitted from the browser cannot be trusted. The thing is that the 'dangerous' data is actually the data that PHP generates (the temporary file name), and this is what has to be protected. That's what my patch did. Now, it doesn't replace userland security checks and correct code. You still have to verify that the upload type was that of a file upload (through the mime type), and then you must use $HTTP_POST_FILES[], instead of using globals, which can be tampered with. Testing that the file resides in the upload tmp dir is also a very good idea. I'll probably post a summary of what the problem was, what code-level fixes were made to PHP and what kind of checks users have to add if they display uploaded files, once we finish analyzing it completely. As for safe-mode - while you were wrong in what you said (as Stas said, it has little to do with the interpreter) - safe mode is indeed falsely advertised as being safe. It's very likely to contain bugs. As far as I'm concerned, it should be clearly advertised as something that would prevent the casual user from doing stuff he's not supposed to do, but isn't suitable for protecting against hackers. Zeev -- Zeev Suraski <zeev@zend.com> http://www.zend.com/