Fix GH-22121: double-free in gdImageSetStyle() after overflow early return#22125
Fix GH-22121: double-free in gdImageSetStyle() after overflow early return#22125iliaal wants to merge 1 commit into
Conversation
|
nice, you probably can target lower branches though. |
@devnexen 100%, given that looks like 8.4 RC is already in-flight should I backport to 8.5 or 8.4 is still ok? |
|
PHP-8.4 please |
iliaal
requested review from
Girgias,
SakiTakamachi,
TimWolla,
adoy,
arnaud-lb,
derickr,
dstogov and
kocsismate
as code owners
iliaal
force-pushed
the
fix/gh-22121-gd-set-style-double-free
branch
from
e45f5f6 to
5277fc6
Compare
iliaal
removed request for
Girgias,
SakiTakamachi,
TimWolla,
adoy,
arnaud-lb,
derickr,
dstogov and
kocsismate
|
Your change is correct but because the array building is very large (i.e. stylearr) it crashes on CI arm. Can you make the test works without such number ? if not, I see there is this possibility eventually |
iliaal
force-pushed
the
fix/gh-22121-gd-set-style-double-free
branch
from
5277fc6 to
d123ddc
Compare
|
Reused |
|
I m not sure I m liking the "workaround", probably @iluuu1994 would have a better idea how to circumvent this issue. |
Open to suggestions, but right now can't think of anything better, and aligns with other tests. |
| } | ||
| } else { | ||
| $memInfo = @file_get_contents("/proc/meminfo"); | ||
| if ($memInfo && preg_match('/MemFree:\s+(\d+) kB/', $memInfo, $matches)) { |
There was a problem hiding this comment.
MemAvailable might be a better fit (MemFree excludes reclaimable page-cache).
| } | ||
| return false; | ||
| } | ||
| if (get_system_memory() < 11 * 1024 * 1024 * 1024) { |
There was a problem hiding this comment.
accounting php startup, 12 GB might be a bit more helpful, wdyt ?
for now, let s keep trying with it. |
that was not a criticism toward you, I just wished we did not need to go to that length for such fix. |
I didn't take it such, feedback is good :) |
…y return gdImageSetStyle freed im->style before checking overflow2(). When the overflow check tripped and the function early-returned, im->style was left dangling. The next gdImageSetStyle, gdImageDestroy, or gdImageSetPixel gdStyled/gdStyledBrushed dispatch then freed or dereferenced it. Move the overflow check above the free to match upstream libgd (libgd/libgd src/gd.c::gdImageSetStyle), which has always had the check first. The original divergence was an oversight in 77ba248 when the overflow check was ported from libgd 2.0.29. Fixes phpGH-22121
iliaal
force-pushed
the
fix/gh-22121-gd-set-style-double-free
branch
from
d123ddc to
a149dce
Compare
2 participants
gdImageSetStyle(ext/gd/libgd/gd.c:2880) freesim->stylebefore callingoverflow2(). When the overflow check trips and the function returns,im->styleis left dangling and the nextgdImageSetStyle,gdImageDestroy, orgdImageSetPixelgdStyled/gdStyledBrusheddispatch frees or dereferences it.Move the overflow check above the free to match upstream libgd (
libgd/libgdsrc/gd.c::gdImageSetStyle), which has always had the check first. The divergence was an oversight in 77ba248 when the overflow check was ported from libgd 2.0.29.Fixes #22121