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

At 01:16 07/09/2000, Jon Ribbens wrote:

Zeev Suraski <zeev@zend.com> wrote: The difference is in the approach. You started off by whining. If that's what you wish to do, I'll appreciate it if you do it off the mailing list. Well, darn it, what's the point of that?

There's no point in bashing at all, because it won't get you anywhere. So if you just wish to annoy people, we don't care to see it.

Constructive criticism is something completely different from whining or bashing others. Both can be done simultaneously, however. There seem to be a lot of people here failing to notice this. You can ignore me if you like, but the points I am raising won't go away.

No, they can't. I'm not sure how old you are, but it seems you have very very little experience with people.

(binary safety to avoid buffer overflows,

I found this interesting undocumented feature while looking at all this file upload stuff: /* FIXME: XXX: not binary safe, discards returned length */

It seems to me more and more that you're not serious - all you seem to do all the time is whine. That's funny. I thought I just pointed out a bug. What's your definition of 'whine' then? Someone who doesn't agree with you?

You didn't point out a bug. Binary safety is a guideline that helps security, not a higher law without which your code is buggy. I guess you don't even know that. In the particular place you pointed out, the lack of binary safety: a. Probably has no implications at all. b. Definitely has no security implications Sorry to move you off your wiseguy status for this one.

You said that PHP 4 had binary safety, I mentioned out that in the very small amount of PHP 4 source I have read, I can immediately point to an important instance where that simply isn't true. If you don't want me to disagree with you, choose what you say more carefully.

Correct me if I'm wrong, but my assumption is that you grep'd for FIXME, rather than read the code. Reading through code doesn't sound like something you would do.

As for the issue itself - I'm not sure if we're expecting binary data here (I'm pretty sure we're not). I'm sure you could guess that I would say this but: if this is the case, then you need to document it.

Huh? Why? It's a low level implementation thing, something that's closed in the level of source-level comments. You want to start arguing about commenting? I guess we could. Oh my God, does PHP have thousands of comment 'bugs'. You may have found one of them. Perhaps you should post it to bugtraq?

Considering the huge userbase and the fact nobody complained so far, it sounds like you're even whining about the wrong things. You brought the issue up, not me.

I brought the issue of PHP's core having been programmed with security in mind. I gave examples. You took one of my examples, and applied it without too much skill and knowledge to an innocent FIXME in the PHP source code. There are plenty of other places in PHP which don't use binary safety, but rather, C-style strings. It doesn't mean they're not safe. Again, binary safety helps, it's not mandatory.

Trust me, I know exactly what my patch does and what it doesn't do (and I knew that before you pointed it out so wisely). Security wise, it doesn't do much. So why did you post it as a reply to the advisory on BUGTRAQ?

Because at the time I was wrong, and thought it would give a higher degree of security. Granted, I was stuck in thinking in the same pattern of the earlier fix by Rasmus, I just made that fix work reliably. As I said, he who doesn't do, doesn't go wrong. If you haven't figured out what that means yet in this context - I do a lot, and thus I also go wrong sometimes. We'll publish a new answer soon.

Excellent. Put it in the manual! Make the next release have it turned off by default! For sure, put in really large letters in the release notes what you've done, but new users and new scripts should not continue to be written using register_globals.

Breaking 99.9% of the scripts in the world isn't my idea of fun, You have such little faith in your users that you don't think there is any way you can get them to realise that the default has changed, and to change it back if they need it? As a PHP user, I should be insulted.

I could care less if you personally got insulted. However, to be blunt, no, I don't have faith in my users, if that's how you want to put it (another way to say it is that it has nothing to do with faith in users). Undoubtfully, this would cause many people a great deal of pain and lost hours of work.

Make them keywords. Use a special '%' syntax. Whatever. What you say here is simply untrue.

I might be funny here, but I kinda trust what I have to say about the subject more than I trust what you have to say about the subject. Using functions to access globals is extremely slow, and thus completely silly (c). Use one letter variables if you wish, just don't use functions. If you say functions are slower, then fine, I believe you. Which is why I listed several alternatives to achieve the same effect above. But you seem to have ignored that.

What, start cluttering the language syntax with new constructs? Of course I ignored that. Why the hell would we want to do that? Perlize ourselves? Use one letter variable names if you wish, and stop reinventing the world all the time.

If that tone is going to continue, this is the last time I'll converse with you. I don't see why this should be the case, but that's your choice. It's your loss more than mine. But again, this post is guaranteed 100% pure sweetness-and-light. Or your money back.

I'm willing to risk that great loss. I have no respect for people who have no respect for me, and I don't converse with people I don't respect. Zeev -- Zeev Suraski <zeev@zend.com> http://www.zend.com/