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

Zeev Suraski <zeev@zend.com> wrote: > 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. What, exactly, do you think your patch did? So far as I can see it makes it so that, when using multiport/form-data POSTs *only*, you cannot overwrite the PHP-generated variables with other POST variables. Why is this useful? Why does the attacker simply not upload any files in their POST, and thus be free to set whatever variables they like? > 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. So it appears that the fix was there all along - use the undocumented feature HTTP_POST_FILES. Why do you need any change to the source at all? All you need to fix is the manual - document HTTP_POST_FILES and explain in the file upload documentation that you should not access uploaded files in any other way. Since this is what you still have to to even after the patch, and it would have worked before, what has the patch bought anyone? > 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. Yes. Documentation is *important*. > As for safe-mode - while you were wrong in what you said (as Stas said, it > has little to do with the interpreter) Sorry, I was simply meaning 'the PHP system'. It looks like an interpreter usually. > - 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. Again, documentation is important. If safe_mode is described as above, then it is not broken or buggy. Since it is actually almost entirely undocumented, it *is* broken and a health hazard. Cheers Jon