Skip to content

Fix memory leak in shm_get_var() when variable is corrupted #21388

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ndossche wants to merge 2 commits into from
+42 −3
Closed

Fix memory leak in shm_get_var() when variable is corrupted #21388

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
13c45ef
Fix memory leak in shm_get_var() when variable is corrupted
ndossche Mar 8, 2026
9aec1ac
fixup! skipifs
ndossche Mar 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions ext/sysvshm/sysvshm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,11 +309,13 @@ PHP_FUNCTION(shm_get_var)
shm_data = &shm_var->mem;

PHP_VAR_UNSERIALIZE_INIT(var_hash);
if (php_var_unserialize(return_value, (const unsigned char **) &shm_data, (unsigned char *) shm_data + shm_var->length, &var_hash) != 1) {
int res = php_var_unserialize(return_value, (const unsigned char **) &shm_data, (unsigned char *) shm_data + shm_var->length, &var_hash);
PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
Copy link
Member

@devnexen devnexen Mar 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible to minimise the change (i.e. keeping RETVAL_FALSE) ?

Copy link
Member Author

@ndossche ndossche Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Someone else is going to change it to RETURN_FALSE on master anyway, so might as well already do it

if (res != 1) {
php_error_docref(NULL, E_WARNING, "Variable data in shared memory is corrupted");
RETVAL_FALSE;
zval_ptr_dtor(return_value);
RETURN_FALSE;
}
PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
}
/* }}} */

Expand Down
37 changes: 37 additions & 0 deletions ext/sysvshm/tests/shm_get_var_leak.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
--TEST--
shm_get_var() leaks if variable is corrupted
--EXTENSIONS--
sysvshm
ffi
--INI--
ffi.enable=1
--SKIPIF--
<?php
if (!function_exists('ftok')) die('skip needs ftok');
if (PHP_INT_SIZE !== 8) die('skip only for 64-bit');
if (PHP_OS_FAMILY !== 'Linux') die('skip only for decent operating systems');
?>
--FILE--
<?php

$key = ftok(__FILE__, 't');
$s = shm_attach($key, 128);

shm_put_var($s, 0, [1, 2]);

$ffi = FFI::cdef(<<<CODE
int shmget(int, size_t, int);
char *shmat(int, const void *, int);
CODE);
$ptr = $ffi->shmat($ffi->shmget($key, 0, 0), $ffi->new('void *'), 0);

$ptr[0x40 + 13] = 0; // Corrupt first byte of second element of serialized data

var_dump(shm_get_var($s, 0));

shm_remove($s);

?>
--EXPECTF--
Warning: shm_get_var(): Variable data in shared memory is corrupted in %s on line %d
bool(false)
Loading