True, that bit of code made no sense and has been fixed.  The entire thing has been reworked for the 4.2 tree, but if you could expand on the muriad of buffer overflows aside from the memchr()+1 mixup, and submit a useful bug report it would be appreciated.

It what way is it "fixed"? Every PHP user in the entire world is going to have to download the patches from www.php.net to fix the security hole, and those patches contain this bug. I know that it is fixed in CVS in that the entire file has been replaced, but as I understand it there is no fixed release version.

As to the other bugs, just look at the main while() loop in php_mime_split(). Pretty much every call to str* functions (including the very first one) are reading beyond the end of the buffer. If this happens, 'rem' may become negative and even more excitement ensues.

The specific memchr()+1 issue is fixed in CVS which was the only useful part of this bug report.  We close bugs when they are fixed in CVS, not when we ship releases.  

Fine by me, but the problems are not fixed in CVS. You asked me for more specifics, I gave them to you.

Your claims are simply wrong.

Not a single str* function is able to read beyond the
buffer, cause the buffer is '\0' terminated and
strcmp/strcasecmp whatever will stop there.

We can search and fix what's wrong if there is a bug description, but it would nice if you could post patch to php-dev directly.  We know PHP has many bugs and appreciate patches fixes bugs.

You have skills, right :)

I'll admit that I did not examine the rest of the program to see if the buffer was '\0'-terminated, however if it is, it's not just me that thought it wasn't - whoever wrote the routine thought it wasn't either. Otherwise there wouldn't even be any point in passing the buffer length to the function, or the main loop's "while (ptr - buf < cnt)" or indeed half the function.

As to providing patches, I know from experience that what you tend to do with them is ignore them, insult them, re-write them badly and apply them six months later, and then fail to credit. Plus I see no point in providing band-aids in a futile attempt to cover the gaping wounds in PHP. I *can* give you the fix I recommend to people for PHP, however, which is 'rm -rf php-*' ;-)

You are again wrong, cnt must be supplied.
I advise you to think before you speak.

A POST fileupload block can have lots of '\0's in it.
Without the number of bytes it would be impossibe to
handle such a block.

How about this patch:

--- main/rfc1867.c.orig Thu Feb 28 14:08:25 2002
+++ main/rfc1867.c      Thu Feb 28 14:33:03 2002
@@ -163,20 +163,28 @@
                                                SAFE_RETURN;
                                        }
                                        /* some other headerfield found, skip it */
-                                       loc = (char *) memchr(ptr, '\n', rem)+1;
+                                       loc = (char *) memchr(ptr, '\n', rem);
                                        if (!loc) {
                                                /* broken */
                                                php_error(E_WARNING, "File Upload Mime headers garbled ptr: [%c%c%c%c%c]", *ptr, *(ptr + 1), *(ptr + 2), *(ptr
+ 3), *(ptr + 4));
                                                SAFE_RETURN;
                                        }
+                                       else
+                                       {
+                                           loc++;
+                                       }
                                        while (*loc == ' ' || *loc == '\t') {
                                                /* other field is folded, skip it */
-                                               loc = (char *) memchr(loc, '\n', rem-(loc-ptr))+1;
+                                               loc = (char *) memchr(loc, '\n', rem-(loc-ptr));
                                                if (!loc) {
                                                        /* broken */
                                                        php_error(E_WARNING, "File Upload Mime headers garbled ptr: [%c%c%c%c%c]", *ptr, *(ptr + 1), *(ptr +
2), *(ptr + 3), *(ptr + 4));
                                                        SAFE_RETURN;
                                                }
+                                               else
+                                               {
+                                                   loc++;
+                                               }
                                        }
                                        rem -= (loc - ptr);
                                        ptr = loc;
@@ -232,6 +240,10 @@
                                         * pre 4.0.6 code here
                                         */
                                        loc2 = memchr(loc + 1, '\n', rem);
+                                       if (!loc2) {
+                                               php_error(E_WARNING, "File Upload Mime headers - no newline");
+                                               SAFE_RETURN;
+                                       }
                                        rem -= (loc2 - ptr) + 1;
                                        ptr = loc2 + 1;
                                        /* is_arr_upload is true when name of file upload field

I have had a long look at rfc1867.c v 1.71.2.2 2002/02/21
from a download of php4.1.2 today (1 Mar 10:00 CET). There are a large number of dubious cases of handling of the buffer being processed. The following diffs address most of these (I believe). I am posting the patches to the php-dev list, since it's difficult if not impossible to create a properfly formatted diff in this edit window.

Fuck you ...php

> Fuck you ...php

This posting is most probably a fake, cause there is
noone at heik@netcraft.com

And for the rest of the trolls:

The patch from jes@nl.demon.net will not be applied.
All his claims were as bogus as his patch.
He just added lots of redundant code. And the best:
In his patch every single variable is double freed.
You know how dangerous that is...

Folks, please, we need a working fix, stop bitching at each other.

Is the official response from the PHP team that "the fix is in CVS, so it is fixed"?

Would it be possible to post the IP addresses of the users of php.net?  Some would very much appreciate that.

This would be a violation of (at least german) userdata protection laws.

Make that european...

Derick

I note that 'adrianc@e-smith.com' submitted another patch that seems to fix the issue.  What about that patch?

The current patch fixes the exploit.  Yes, there is a memchr() crash bug left in it which is fixed in CVS, and the 4.2 code has been completely rewritten.  Basically the issue has been addressed.