Bug #69619 [Com]: wrong argument for memcpy in from_zval_write_control

Edit report at https://bugs.php.net/bug.php?id=69619&edit=1 ID: 69619 Comment by: cataphract@php.net Reported by: herumi at nifty dot com Summary: wrong argument for memcpy in from_zval_write_control Status: Feedback Type: Bug Package: Sockets related Operating System: all PHP Version: 5.5.24 Block user comment: N Private report: N New Comment: 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. Previous Comments: ------------------------------------------------------------------------ [2015-05-30 16:15:56] pollita@php.net 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-30 16:07:49] pollita@php.net 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-11 07:19:27] herumi at nifty dot com Description: ------------ 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. cf. https://github.com/php/php-src/blob/master/ext/sockets/conversions.c#L893 Is the correct code as the following? memcpy(&alloc->data, control_buf, sizeof *control_buf); ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=69619&edit=1