ID: 26847
User updated by: nutbar at innocent dot com
Reported By: nutbar at innocent dot com
Status: Open
Bug Type: Mail related
Operating System: any - source code issue
PHP Version: 4.3.4
New Comment:
Ok, maybe I am wrong?
I wrote a PHP script which *should* leak memory if this is indeed not
efree()'ing stuff, but it doesn't seem to.
I noticed it would only potentially (if I was right!) leak ram if say,
the subject was entirely space characters, so I made a script that
called mail() 1000 times roughly, and made a subject that was all
spaces that was 1k long - it should have set me off by 1mb, but instead
I see nothing of the sort.
I'm running the script from the command line (CGI, not CLI though!) if
it makes any difference (I don't believe it does).
Either way, I can't seem to leak the ram, so I guess I was wrong - but
can someone explain to me why it wouldn't? What am I missing here that
wouldn't allow it to leak ram?
Previous Comments:
------------------------------------------------------------------------
[2004-01-08 16:17:25] nutbar at innocent dot com
Here, maybe this will help a bit... Here it assigns values to to_len
and subject_len (among others):
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "sss|ss",
&to,
&to_len,
&subject,
&subject_len,
&message,
&message_len,
&headers,
&headers_len,
&extra_cmd,
&extra_cmd_len
) == FAILURE)
{
return;
}
Then, they check to_len and do stuff if it's greater than 0:
if (to_len > 0) {
to_r = estrndup(to, to_len);
for (; to_len; to_len--) {
if (!isspace((unsigned char) to_r[to_len - 1]))
{
break;
}
to_r[to_len - 1] = '\0';
}
...
Do you see the for loop in there... this one:
for (; to_len; to_len--) {...}
It is modifying to_len itself, which means that to_len, although was
NOT 0 to begin with (and thus, to_r was estrndup()'d and we have to
efree() it later), but IS 0 in the end once the for loop is finished.
Either the for loop must be changed to not modify to_len, or the
efree() statement must be changed to test to_r, not to_len.
Or am I just really out of my mind? I'm not anywhere near as good as
you programmers, but this seems to be sticking out for me quite a bit
(I'm not trying to come off rude! I just think I found something here
and it's not bogus).
------------------------------------------------------------------------
[2004-01-08 16:07:29] nutbar at innocent dot com
I know they check to_len and subject_len - that's not really the
problem.
The problem is that the for loops above that decrement to_len and
subject_len - thus modifying them from their original values.
to_len and subject_len will always be 0, even if they weren't 0 to
begin with. Do you see what I'm referring to?
------------------------------------------------------------------------
[2004-01-08 15:38:41] [EMAIL PROTECTED]
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.
------------------------------------------------------------------------
[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.
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
http://bugs.php.net/26847
--
Edit this bug report at http://bugs.php.net/?id=26847&edit=1
