ID:               26847
 Updated by:       [EMAIL PROTECTED]
 Reported By:      nutbar at innocent dot com
-Status:           Open
+Status:           Bogus
 Bug Type:         Mail related
 Operating System: any - source code issue
 PHP Version:      4.3.4
 New Comment:

Impossible leak. 
        if (to_len > 0) { 
                efree(to_r); 
        } 
        if (subject_len > 0) { 
                efree(subject_r); 
        } 
Is what the code in CVS does. If to_len or subject_len are 
< 1 then no allocation happens in the 1st place. 


Previous Comments:
------------------------------------------------------------------------

[2004-01-08 15:34:23] nutbar at innocent dot com

I guess an alternate fix would also be when the efree()'s are called. 
If you init all your char *'s to NULL, then you can simply do:

if (to_r != NULL) {
     efree(to_r);
}

if (subject_r != NULL) {
     efree(subject_r);
}

------------------------------------------------------------------------

[2004-01-08 15:31:12] nutbar at innocent dot com

By the way, very simple fix:

Add this to the variable declarations:

int j = 0;

Then the lines of code as mentioned before, but fixed:

        if (to_len > 0) {
                to_r = estrndup(to, to_len);

                for (j = to_len; j > 0; j--) {
                        if (!isspace((unsigned char) to_r[j - 1])) {
                                break;
                        }

                        to_r[j - 1] = '\0';
                }



and:

        if (subject_len > 0) {
                subject_r = estrndup(subject, subject_len);

                for (j = subject_len; j > 0; j--) {
                        if (!isspace((unsigned char) subject_r[j - 1]))
{
                                break;
                        }

                        subject_r[j - 1] = '\0';
                }


I just initialized j in the for loop to be the value of to_len and
subject_len, then I use j everywhere rather than to_len or subject_len
so that they stay unmodified.

------------------------------------------------------------------------

[2004-01-08 13:22:55] nutbar at innocent dot com

Description:
------------
In the actual source code for the PHP mail() function
(ext/standard/mail.c), it sets some variables up to hold the To: and
Subject: headers up, and other stuff.

The problem is that if you look at the initial code that checks if the
"to_len" count is greater than 0, it duplicates the "to" string to
"to_r" and does some stuff to it.

It does the same sort of thing with subject_len, subject, and subject_r
in the exact same fashion.

After the new to_r and subject_r strings are used, it goes to free
them, but it does an if () test to see if it should or not - the if
test compares to_len and subject_len to see if they are greater than 0
and if so, efree()'s them.

The problem is that in the code that does stuff with to_r and
subject_r, there are for loops which decrement to_len and subject_len
so it can walk the strings.  By doing this, you bring the to_len and
subject_len variables to 0, thus nothing is ever efree()'d in the end,
and you've got a memory leak.

The leak is small and not noticable typically, but with mass mailing
scripts that loop using mail(), it could be huge.

Reproduce code:
---------------
See mail.c - lines 106 to 113, 129 to 136, and then 160 to 165.

Actual result:
--------------
I have not tested for an actual memory leak by calling mail() in a loop
- I was just going to write my own mail() function and was using the
code in mail.c to do it with, and came across this.

If this is a false report, I am sorry, but I do believe it's real.


------------------------------------------------------------------------


-- 
Edit this bug report at http://bugs.php.net/?id=26847&edit=1

Reply via email to