|  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #69619 wrong argument for memcpy in from_zval_write_control
Submitted: 2015-05-11 07:19 UTC Modified: 2015-06-17 20:36 UTC
Avg. Score:5.0 ± 0.0
Reproduced:0 of 1 (0.0%)
From: herumi at nifty dot com Assigned: pollita (profile)
Status: Closed Package: Sockets related
PHP Version: 5.5.24 OS: all
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
Block user comment
Status: Assign to:
Bug Type:
From: herumi at nifty dot com
New email:
PHP Version: OS:


 [2015-05-11 07:19 UTC] herumi at nifty dot com
The code ext/sockets/conversion.c:914 in php-5.5.24

if (space_left < req_space) {
    *control_buf = safe_erealloc(*control_buf, 2, req_space, *control_len);
    *control_len += 2 * req_space;
    memset(*control_buf, '\0', *control_len - *offset);      // (A)
    memcpy(&alloc->data, *control_buf, sizeof *control_buf); // (B)

Maybe th code (B) fill alloc->data with 8-byte zeros
because (A) fills *control_buf with zeros,
then the value of *control_buf is lost.


Is the correct code as the following?
memcpy(&alloc->data, control_buf, sizeof *control_buf);


fix_from_zval_write_control.diff (last revision 2015-05-11 07:19 UTC by herumi at nifty dot com)

Add a Patch

Pull Requests

Add a Pull Request


AllCommentsChangesGit/SVN commitsRelated reports
 [2015-05-30 16:07 UTC]
-Status: Open +Status: Feedback
 [2015-05-30 16:07 UTC]
Looking at the code, your analysis certainly seems to be correct, and I'm happy enough to apply the fix (including to prior supported branches), but I'd like to include a regression test.  Do you have some reproduce code for this?

I realize that the bug presents itself in the form of a leak, but developer builds usually have --enable-debug turned on which produce leak warnings, so any repro which triggers the leak should properly fail any test case which uses this code path.
 [2015-05-30 16:15 UTC]
In fact, that memset feels wrong as well.  I would assume it wants to clear the >end< of the buffer, not the beginning.  Hence:

memset(*control_buf + *offset, '\0', *control_len - *offset);
 [2015-05-31 22:45 UTC]
I agree with everything.

The memcpy should be copying the value of the reallocated pointer, not the data it points to and in fact the memset looks suspicious from the onset because you usually see memcpy(&foo, &bar, sizeof(bar)), i.e., one extra level of indirection in the 2nd argument.

And yes the memset looks like it should start at ((char*)*control_buf) + *offset) (zero the newly allocated segment).

I guess this branch was never exercised in the tests. Sorry for the trouble.
 [2015-06-07 04:22 UTC] php-bugs at lists dot php dot net
No feedback was provided. The bug is being suspended because
we assume that you are no longer experiencing the problem.
If this is not the case and you are able to provide the
information that was requested earlier, please do so and
change the status of the bug back to "Re-Opened". Thank you.
 [2015-06-07 13:59 UTC]
-Status: No Feedback +Status: Open
 [2015-06-13 02:20 UTC]
-Assigned To: +Assigned To: pollita
 [2015-06-13 02:20 UTC]
Thanks for confirming my thoughts! I'll merge this soon (unless you get to it first)
 [2015-06-17 20:36 UTC]
-Status: Assigned +Status: Closed
 [2015-06-17 20:36 UTC]
The fix for this bug has been committed.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at

 For Windows:
Thank you for the report, and for helping us make PHP better.

PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sat Jul 20 23:01:28 2024 UTC