# Memory corruption bug

- Thread: https://externals.io/message/44493
- Original archive: https://news-web.php.net/php.internals/44493
- Started by Andrei Zmievski on 2009-06-26T21:23:15+00:00
- Messages: 8
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt

---

## #44493 — Andrei Zmievski — 2009-06-26T21:23:15+00:00

URL: https://externals.io/message/44493#44493

I found what is effectively a memory corruption bug in all the branches. pecalloc() uses 
the wrong length to zero out the memory. Patch is attached, although I'm somewhat 
concerned about using just (nmemb*len) instead of something like safe_address(nmemb*len), 
but safe_address() is inlined in zend_alloc.c not in the header file.

We should apply this to 5.2/5.3 before the release.

-Andrei

---

## #44496 — Stanislav Malyshev — 2009-06-26T22:00:45+00:00

URL: https://externals.io/message/44493#44496 · In reply to: #44493

Hi!

> pecalloc() uses the wrong length to zero out the memory. Patch is 
> attached, although I'm somewhat concerned about using just (nmemb*len) 
> instead of something like safe_address(nmemb*len), but safe_address() is 
> inlined in zend_alloc.c not in the header file.

You just did safe_address in _safe_malloc(nmemb, len, 0) which should 
have called `E_ERROR` if nmemb*len overflows, so do you need to do it again?
Leaving a comment about it wouldn't hurt though :)
-- 
Stanislav Malyshev, Zend Software Architect
stas@zend.com   http://www.zend.com/
(408)253-8829   MSN: stas@zend.com

---

## #44498 — Andrei Zmievski — 2009-06-26T22:15:34+00:00

URL: https://externals.io/message/44493#44498 · In reply to: #44496

Stanislav Malyshev wrote:
> You just did safe_address in _safe_malloc(nmemb, len, 0) which should 
> have called `E_ERROR` if nmemb*len overflows, so do you need to do it again?
> Leaving a comment about it wouldn't hurt though :)

Yeah, you're right, but I didn't write the code. :) Regardless, I think this fix should go 
into 5.2/5.3 because calloc() advertises memory clearing as part of its API contract.

-Andrei

---

## #44500 — Pierre Joye — 2009-06-26T23:17:00+00:00

URL: https://externals.io/message/44493#44500 · In reply to: #44498

5.2 is already released so you can commit anyway.

For 5.3, I'm not sure (well I am), but wait until Johannes and Lukas
are back online please.


> Stanislav Malyshev wrote:
> > […]
>
> Yeah, you're right, but I didn't write the code. :) Regardless, I think this
> fix should go into 5.2/5.3 because calloc() advertises memory clearing as
> part of its API contract.
>
> -Andrei
>
> --
> 
> 
>
>



-- 
Pierre

http://blog.thepimp.net | http://www.libgd.org

---

## #44517 — johannes@php.net — 2009-06-27T11:12:41+00:00

URL: https://externals.io/message/44493#44517 · In reply to: #44493

> I found what is effectively a memory corruption bug in all the branches. pecalloc() uses 
> the wrong length to zero out the memory. Patch is attached, although I'm somewhat 
> concerned about using just (nmemb*len) instead of something like safe_address(nmemb*len), 
> but safe_address() is inlined in zend_alloc.c not in the header file.
> 
> We should apply this to 5.2/5.3 before the release.

Agreed.

johanes

---

## #44527 — Pierre Joye — 2009-06-28T10:10:44+00:00

URL: https://externals.io/message/44493#44527 · In reply to: #44517

committed.


2009/6/27 Johannes Schlüter <johannes@php.net>:
> 
> > […]
>
> Agreed.
>
> johanes
>
>
>
> --
> 
> 
>
>



-- 
Pierre

http://blog.thepimp.net | http://www.libgd.org

---

## #44528 — Andrei Zmievski — 2009-06-28T17:59:31+00:00

URL: https://externals.io/message/44493#44528 · In reply to: #44527

Pierre Joye wrote:
> committed.

I see that it's not in HEAD though, probably because HEAD has never been updated to use 
safe-allocation functions? I see no trace of __zend_calloc() there. We should probably 
port that from 5.3.

-Andrei

---

## #44530 — Pierre Joye — 2009-06-28T21:02:10+00:00

URL: https://externals.io/message/44493#44530 · In reply to: #44528

2009/6/28 Andrei Zmievski <andrei@gravitonic.com>:
> Pierre Joye wrote:
> > […]
>
> I see that it's not in HEAD though, probably because HEAD has never been
> updated to use safe-allocation functions? I see no trace of __zend_calloc()
> there. We should probably port that from 5.3.

Yes, noticed that too while committing them. Thanks for having added
them to the TODO :)

Cheers,
-- 
Pierre

http://blog.thepimp.net | http://www.libgd.org