[sysvshm] Clamp shm_get_var chunk length to segment bounds

[iliaal]

shm_attach() validated only the "PHP_SM" magic and then took ptr->start, ptr->end, ptr->free and ptr->total from the segment itself, so every later bound came from attacker-controlled data rather than the size shmctl(IPC_STAT) reports. end = 1<<30 in a 4096-byte mapping walks php_var_unserialize() off the end in shm_get_var(), a negative start does the same to php_check_shm_data() from shm_has_var(), and that same end makes memcpy_len large and positive so shm_remove_var() memmoves out of bounds. All three segfault on unpatched PHP-8.4. The header is now validated against the kernel-reported size at attach, and php_remove_shm_data() bounds the chunk it unlinks.

The previous description claimed php_remove_shm_data() was unaffected because it only memmoves when memcpy_len > 0. That was wrong: a corrupt ptr->end makes memcpy_len large and positive. The three segment layouts devnexen posted are now the regression tests; they use shmop rather than FFI, so they run wherever shmop does.

[devnexen]

can we have this test ?

--TEST--
shm_get_var() must not trust the chunk length stored in the segment
--EXTENSIONS--
sysvshm
ffi
--INI--
ffi.enable=1
--SKIPIF--
<?php
if (PHP_OS_FAMILY !== 'Linux') die('skip Linux only');
if (PHP_INT_SIZE !== 8) die('skip 64-bit only');
?>
--FILE--
<?php
$key = ftok(__FILE__, 't');

$ffi = FFI::cdef("
    typedef struct { char magic[8]; long start; long end; long free_; long total; } head_t;
    typedef struct { long key; long length; long next; char mem[8]; } chunk_t;
    int shmget(int, size_t, int);
    void *shmat(int, const void *, int);
    int shmdt(const void *);
");

$p = $ffi->shmat($ffi->shmget($key, 4096, 0666 | 01000), null, 0);
FFI::memset($p, 0, 4096);

$head = $ffi->cast('head_t*', $p);
FFI::memcpy($head->magic, "PHP_SM", 6);
$head->start = 40;
$head->end = 80;
$head->free_ = 4016;
$head->total = 4096;

$chunk = $ffi->cast('chunk_t*', $ffi->cast('char*', $p) + 40);
$chunk->key = 1;
$chunk->next = 40;
$chunk->length = 1 << 20;
FFI::memcpy($chunk->mem, "i:42;", 5);

$ffi->shmdt($p);

$shm = shm_attach($key, 4096);
var_dump(shm_get_var($shm, 1));
shm_remove($shm);
?>
--EXPECTF--
Warning: shm_get_var(): Variable data in shared memory is corrupted in %s on line %d
bool(false)

[iliaal]

That's already shm_get_var_chunk_length_bounds.phpt.

[devnexen]

right right, copied the wrong tests. took the opportunity to add a few more then.

--TEST--
sysvshm: shm_attach() must reject a segment whose header is out of bounds
--EXTENSIONS--
sysvshm
shmop
--SKIPIF--
<?php
if (PHP_INT_SIZE !== 8) die('skip 64-bit only');
?>
--FILE--
<?php
$key = ftok(__FILE__, 's');

$old = @shmop_open($key, 'w', 0, 0);
if ($old !== false) { shmop_delete($old); }

$seg = 4096;
$h = shmop_open($key, 'c', 0600, $seg);

shmop_write($h, pack('a8qqqq', 'PHP_SM', 40, 1 << 30, 0, $seg), 0);
shmop_write($h, pack('qqq', 1, 1 << 20, 32) . 's:1000000:"', 40);

$shm = shm_attach($key, $seg);
var_dump($shm);
if ($shm !== false) {
    var_dump(shm_get_var($shm, 1));
    shm_remove($shm);
}
echo "Done\n";
?>
--EXPECTF--
Warning: shm_attach(): Failed for key 0x%x: segment header is corrupted in %s on line %d
bool(false)
Done
--CLEAN--
<?php
$key = ftok(__FILE__, 's');
$h = @shmop_open($key, 'w', 0, 0);
if ($h !== false) { shmop_delete($h); }
?>

--TEST--
sysvshm: shm_has_var() must reject a segment whose start offset is out of bounds
--EXTENSIONS--
sysvshm
shmop
--SKIPIF--
<?php
if (PHP_INT_SIZE !== 8) die('skip 64-bit only');
?>
--FILE--
<?php
$key = ftok(__FILE__, 's');
$old = @shmop_open($key, 'w', 0, 0);
if ($old !== false) { shmop_delete($old); }

$seg = 4096;
$h = shmop_open($key, 'c', 0600, $seg);
shmop_write($h, pack('a8qqqq', 'PHP_SM', -(1 << 40), 4096, 0, $seg), 0);

$shm = shm_attach($key, $seg);
var_dump($shm);
if ($shm !== false) {
    var_dump(shm_has_var($shm, 1));
    shm_remove($shm);
}
echo "Done\n";
?>
--EXPECTF--
Warning: shm_attach(): Failed for key 0x%x: segment header is corrupted in %s on line %d
bool(false)
Done
--CLEAN--
<?php
$key = ftok(__FILE__, 's');
$h = @shmop_open($key, 'w', 0, 0);
if ($h !== false) { shmop_delete($h); }
?>

--TEST--
sysvshm: shm_remove_var() must not memmove past a segment with a corrupt end
--EXTENSIONS--
sysvshm
shmop
--SKIPIF--
<?php
if (PHP_INT_SIZE !== 8) die('skip 64-bit only');
?>
--FILE--
<?php
$key = ftok(__FILE__, 's');
$old = @shmop_open($key, 'w', 0, 0);
if ($old !== false) { shmop_delete($old); }

$seg = 4096;
$h = shmop_open($key, 'c', 0600, $seg);
shmop_write($h, pack('a8qqqq', 'PHP_SM', 40, 1 << 30, 0, $seg), 0);
shmop_write($h, pack('qqq', 1, 0, 32), 40);

$shm = shm_attach($key, $seg);
var_dump($shm);
if ($shm !== false) {
    shm_remove_var($shm, 1);
    shm_remove($shm);
}
echo "Done\n";
?>
--EXPECTF--
Warning: shm_attach(): Failed for key 0x%x: segment header is corrupted in %s on line %d
bool(false)
Done
--CLEAN--
<?php
$key = ftok(__FILE__, 's');
$h = @shmop_open($key, 'w', 0, 0);
if ($h !== false) { shmop_delete($h); }
?>

[iliaal]

Ah, all three crash, and my clamp missed all three: it derived its bound from ptr->end, which is the field your first layout corrupts. shm_attach() now validates start/end/free/total against the size shmctl(IPC_STAT) reports, and php_remove_shm_data() bounds the chunk it unlinks. Your three tests are in as-is, including the "segment header is corrupted" warning, which did not exist before this.