Skip to content

Commit d848473

Browse files
iliaaliliaal
committed
[sysvshm] Validate segment header and chunk bounds before use
shm_attach() took ptr->start, ptr->end, ptr->free and ptr->total on trust once the "PHP_SM" magic matched, and every later bound came from those segment-resident fields rather than the size shmctl(IPC_STAT) reports, so a segment claiming a huge end or a negative start walked php_check_shm_data(), php_var_unserialize() and the memmove() in php_remove_shm_data() off the mapping. Validate the header against the kernel size at attach time, keep the chunk length check in shm_get_var(), and bound the chunk being unlinked so a corrupt next cannot drive the move. Closes GH-23495
1 parent 35b09ea commit d848473

6 files changed

Lines changed: 244 additions & 3 deletions

File tree

‎NEWS‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,10 @@ PHP NEWS
7171
an object converted to an array fails. (David Carlier)
7272
. Fixed read buffer compaction in php_stream_filter_flush(). (crystarm)
7373

74+
- Sysvshm:
75+
. Fixed out-of-bounds reads and writes when a shared memory segment carries
76+
a corrupted header or chunk length. (iliaal)
77+
7478
- Zip:
7579
. Fixed bug GH-23276 (ZipArchive subclass storing its own stream cannot be
7680
garbage collected). (Weilin Du, ndossche)

‎ext/sysvshm/sysvshm.c‎

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ ZEND_GET_MODULE(sysvshm)
9090
/* TODO: Make this thread-safe. */
9191
sysvshm_module php_sysvshm;
9292

93+
static bool php_check_shm_head(const sysvshm_chunk_head *ptr, zend_long shm_size);
9394
static int php_put_shm_data(sysvshm_chunk_head *ptr, zend_long key, const char *data, zend_long len);
9495
static zend_long php_check_shm_data(sysvshm_chunk_head *ptr, zend_long key);
9596
static int php_remove_shm_data(sysvshm_chunk_head *ptr, zend_long shm_varpos);
@@ -196,6 +197,10 @@ PHP_FUNCTION(shm_attach)
196197
chunk_ptr->end = chunk_ptr->start;
197198
chunk_ptr->total = shm_size;
198199
chunk_ptr->free = shm_size-chunk_ptr->end;
200+
} else if (!php_check_shm_head(chunk_ptr, shm_size)) {
201+
php_error_docref(NULL, E_WARNING, "Failed for key 0x" ZEND_XLONG_FMT ": segment header is corrupted", shm_key);
202+
shmdt(shm_ptr);
203+
RETURN_FALSE;
199204
}
200205

201206
object_init_ex(return_value, sysvshm_ce);
@@ -309,6 +314,8 @@ PHP_FUNCTION(shm_get_var)
309314
sysvshm_shm *shm_list_ptr;
310315
char *shm_data;
311316
zend_long shm_varpos;
317+
zend_long shm_avail;
318+
zend_long shm_len;
312319
sysvshm_chunk *shm_var;
313320
php_unserialize_data_t var_hash;
314321

@@ -331,10 +338,16 @@ PHP_FUNCTION(shm_get_var)
331338
RETURN_FALSE;
332339
}
333340
shm_var = (sysvshm_chunk*) ((char *)shm_list_ptr->ptr + shm_varpos);
341+
shm_avail = shm_list_ptr->ptr->end - shm_varpos - (zend_long) sizeof(sysvshm_chunk);
342+
if (shm_var->length < 0 || shm_var->length > shm_avail) {
343+
php_error_docref(NULL, E_WARNING, "Variable data in shared memory is corrupted");
344+
RETURN_FALSE;
345+
}
346+
shm_len = shm_var->length;
334347
shm_data = &shm_var->mem;
335348

336349
PHP_VAR_UNSERIALIZE_INIT(var_hash);
337-
int res = php_var_unserialize(return_value, (const unsigned char **) &shm_data, (unsigned char *) shm_data + shm_var->length, &var_hash);
350+
int res = php_var_unserialize(return_value, (const unsigned char **) &shm_data, (unsigned char *) shm_data + shm_len, &var_hash);
338351
PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
339352
if (res != 1) {
340353
php_error_docref(NULL, E_WARNING, "Variable data in shared memory is corrupted");
@@ -388,11 +401,33 @@ PHP_FUNCTION(shm_remove_var)
388401
php_error_docref(NULL, E_WARNING, "Variable key " ZEND_LONG_FMT " doesn't exist", shm_key);
389402
RETURN_FALSE;
390403
}
391-
php_remove_shm_data((shm_list_ptr->ptr), shm_varpos);
404+
if (php_remove_shm_data((shm_list_ptr->ptr), shm_varpos) < 0) {
405+
php_error_docref(NULL, E_WARNING, "Variable data in shared memory is corrupted");
406+
RETURN_FALSE;
407+
}
392408
RETURN_TRUE;
393409
}
394410
/* }}} */
395411

412+
/* {{{ php_check_shm_head */
413+
static bool php_check_shm_head(const sysvshm_chunk_head *ptr, zend_long shm_size)
414+
{
415+
if (ptr->total < (zend_long) sizeof(sysvshm_chunk_head) || ptr->total > shm_size) {
416+
return false;
417+
}
418+
if (ptr->start < (zend_long) sizeof(sysvshm_chunk_head) || ptr->start > ptr->total) {
419+
return false;
420+
}
421+
if (ptr->end < ptr->start || ptr->end > ptr->total) {
422+
return false;
423+
}
424+
if (ptr->free < 0 || ptr->free > ptr->total - ptr->end) {
425+
return false;
426+
}
427+
return true;
428+
}
429+
/* }}} */
430+
396431
/* {{{ php_put_shm_data
397432
* inserts an ascii-string into shared memory */
398433
static int php_put_shm_data(sysvshm_chunk_head *ptr, zend_long key, const char *data, zend_long len)
@@ -433,7 +468,7 @@ static zend_long php_check_shm_data(sysvshm_chunk_head *ptr, zend_long key)
433468
pos = ptr->start;
434469

435470
for (;;) {
436-
if (pos >= ptr->end) {
471+
if (ptr->end - pos < (zend_long) sizeof(sysvshm_chunk)) {
437472
return -1;
438473
}
439474
shm_var = (sysvshm_chunk*) ((char *) ptr + pos);
@@ -459,6 +494,11 @@ static int php_remove_shm_data(sysvshm_chunk_head *ptr, zend_long shm_varpos)
459494
ZEND_ASSERT(ptr);
460495

461496
chunk_ptr = (sysvshm_chunk *) ((char *) ptr + shm_varpos);
497+
498+
if (chunk_ptr->next <= 0 || chunk_ptr->next > ptr->end - shm_varpos) {
499+
return -1;
500+
}
501+
462502
next_chunk_ptr = (sysvshm_chunk *) ((char *) ptr + shm_varpos + chunk_ptr->next);
463503

464504
memcpy_len = ptr->end-shm_varpos - chunk_ptr->next;

‎ext/sysvshm/tests/shm_attach_header_bounds.phpt‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
--TEST--
2+
sysvshm: shm_attach() must reject a segment whose header is out of bounds
3+
--EXTENSIONS--
4+
sysvshm
5+
shmop
6+
--SKIPIF--
7+
<?php
8+
if (PHP_INT_SIZE !== 8) die('skip 64-bit only');
9+
?>
10+
--FILE--
11+
<?php
12+
$key = ftok(__FILE__, 's');
13+
14+
$old = @shmop_open($key, 'w', 0, 0);
15+
if ($old !== false) { shmop_delete($old); }
16+
17+
$seg = 4096;
18+
$h = shmop_open($key, 'c', 0600, $seg);
19+
20+
shmop_write($h, pack('a8qqqq', 'PHP_SM', 40, 1 << 30, 0, $seg), 0);
21+
shmop_write($h, pack('qqq', 1, 1 << 20, 32) . 's:1000000:"', 40);
22+
23+
$shm = shm_attach($key, $seg);
24+
var_dump($shm);
25+
if ($shm !== false) {
26+
var_dump(shm_get_var($shm, 1));
27+
shm_remove($shm);
28+
}
29+
echo "Done\n";
30+
?>
31+
--EXPECTF--
32+
Warning: shm_attach(): Failed for key 0x%x: segment header is corrupted in %s on line %d
33+
bool(false)
34+
Done
35+
--CLEAN--
36+
<?php
37+
$key = ftok(__FILE__, 's');
38+
$h = @shmop_open($key, 'w', 0, 0);
39+
if ($h !== false) { shmop_delete($h); }
40+
?>

‎ext/sysvshm/tests/shm_get_var_chunk_length_bounds.phpt‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
--TEST--
2+
shm_get_var() must not trust the chunk length stored in a hostile segment for unserialize()
3+
--EXTENSIONS--
4+
sysvshm
5+
ffi
6+
--INI--
7+
ffi.enable=1
8+
--SKIPIF--
9+
<?php
10+
if (PHP_OS_FAMILY !== 'Linux') {
11+
die('skip shmget is not available to FFI on this OS');
12+
}
13+
if (PHP_INT_SIZE !== 8) {
14+
die('skip FFI chunk layout assumes 64-bit zend_long');
15+
}
16+
?>
17+
--FILE--
18+
<?php
19+
20+
function craft_hostile_segment(int $key, int $len): void {
21+
$ffi = FFI::cdef("
22+
typedef struct { char magic[6]; long start; long end; long free_; long total; } head_t;
23+
typedef struct { long key; long length; long next; char mem[16]; } chunk_t;
24+
int shmget(int, int, int);
25+
void *shmat(int, const void *, int);
26+
");
27+
$id = $ffi->shmget($key, 4096, 0666 | 01000);
28+
if ($id < 0) {
29+
echo "shm setup failed\n";
30+
return;
31+
}
32+
$p = $ffi->shmat($id, NULL, 0);
33+
if ($p == $ffi->cast('char*', -1)) {
34+
echo "shmat failed\n";
35+
return;
36+
}
37+
FFI::memset($p, 0, 4096);
38+
$head = $ffi->cast('head_t*', $p);
39+
FFI::memcpy($head->magic, "PHP_SM", 6);
40+
$head->start = 40;
41+
$head->end = 4096;
42+
$head->free_ = 0;
43+
$head->total = 4096;
44+
$chunk = $ffi->cast('chunk_t*', $ffi->cast('char*', $p) + 40);
45+
$chunk->key = 1;
46+
$chunk->length = $len;
47+
$chunk->next = 4096 - 40;
48+
FFI::memcpy($chunk->mem, "i:42;", 5);
49+
}
50+
51+
$key1 = 0x5A5A0E01;
52+
$key2 = 0x5A5A0E02;
53+
54+
$old = @shm_attach($key1);
55+
if ($old !== false) {
56+
shm_remove($old);
57+
}
58+
$old = @shm_attach($key2);
59+
if ($old !== false) {
60+
shm_remove($old);
61+
}
62+
craft_hostile_segment($key1, PHP_INT_MAX);
63+
64+
$shm = shm_attach($key1, 4096);
65+
var_dump(shm_has_var($shm, 1));
66+
var_dump(shm_get_var($shm, 1));
67+
shm_remove($shm);
68+
69+
$shm2 = shm_attach($key2, 4096);
70+
shm_put_var($shm2, 1, 42);
71+
var_dump(shm_get_var($shm2, 1));
72+
shm_remove($shm2);
73+
74+
echo "Done\n";
75+
?>
76+
--EXPECTF--
77+
bool(true)
78+
79+
Warning: shm_get_var(): Variable data in shared memory is corrupted in %s on line %d
80+
bool(false)
81+
int(42)
82+
Done

‎ext/sysvshm/tests/shm_has_var_start_bounds.phpt‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
--TEST--
2+
sysvshm: shm_has_var() must reject a segment whose start offset is out of bounds
3+
--EXTENSIONS--
4+
sysvshm
5+
shmop
6+
--SKIPIF--
7+
<?php
8+
if (PHP_INT_SIZE !== 8) die('skip 64-bit only');
9+
?>
10+
--FILE--
11+
<?php
12+
$key = ftok(__FILE__, 's');
13+
$old = @shmop_open($key, 'w', 0, 0);
14+
if ($old !== false) { shmop_delete($old); }
15+
16+
$seg = 4096;
17+
$h = shmop_open($key, 'c', 0600, $seg);
18+
shmop_write($h, pack('a8qqqq', 'PHP_SM', -(1 << 40), 4096, 0, $seg), 0);
19+
20+
$shm = shm_attach($key, $seg);
21+
var_dump($shm);
22+
if ($shm !== false) {
23+
var_dump(shm_has_var($shm, 1));
24+
shm_remove($shm);
25+
}
26+
echo "Done\n";
27+
?>
28+
--EXPECTF--
29+
Warning: shm_attach(): Failed for key 0x%x: segment header is corrupted in %s on line %d
30+
bool(false)
31+
Done
32+
--CLEAN--
33+
<?php
34+
$key = ftok(__FILE__, 's');
35+
$h = @shmop_open($key, 'w', 0, 0);
36+
if ($h !== false) { shmop_delete($h); }
37+
?>

‎ext/sysvshm/tests/shm_remove_var_end_bounds.phpt‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
sysvshm: shm_remove_var() must not memmove past a segment with a corrupt end
3+
--EXTENSIONS--
4+
sysvshm
5+
shmop
6+
--SKIPIF--
7+
<?php
8+
if (PHP_INT_SIZE !== 8) die('skip 64-bit only');
9+
?>
10+
--FILE--
11+
<?php
12+
$key = ftok(__FILE__, 's');
13+
$old = @shmop_open($key, 'w', 0, 0);
14+
if ($old !== false) { shmop_delete($old); }
15+
16+
$seg = 4096;
17+
$h = shmop_open($key, 'c', 0600, $seg);
18+
shmop_write($h, pack('a8qqqq', 'PHP_SM', 40, 1 << 30, 0, $seg), 0);
19+
shmop_write($h, pack('qqq', 1, 0, 32), 40);
20+
21+
$shm = shm_attach($key, $seg);
22+
var_dump($shm);
23+
if ($shm !== false) {
24+
shm_remove_var($shm, 1);
25+
shm_remove($shm);
26+
}
27+
echo "Done\n";
28+
?>
29+
--EXPECTF--
30+
Warning: shm_attach(): Failed for key 0x%x: segment header is corrupted in %s on line %d
31+
bool(false)
32+
Done
33+
--CLEAN--
34+
<?php
35+
$key = ftok(__FILE__, 's');
36+
$h = @shmop_open($key, 'w', 0, 0);
37+
if ($h !== false) { shmop_delete($h); }
38+
?>

0 commit comments

Comments
 (0)