diff --git a/NEWS b/NEWS index 519b0ccaf053..e41a0227d7f6 100644 --- a/NEWS +++ b/NEWS @@ -71,6 +71,10 @@ PHP NEWS an object converted to an array fails. (David Carlier) . Fixed read buffer compaction in php_stream_filter_flush(). (crystarm) +- Sysvshm: + . Fixed out-of-bounds reads and writes when a shared memory segment carries + a corrupted header or chunk length. (iliaal) + - Zip: . Fixed bug GH-23276 (ZipArchive subclass storing its own stream cannot be garbage collected). (Weilin Du, ndossche) diff --git a/ext/sysvshm/sysvshm.c b/ext/sysvshm/sysvshm.c index 9c68c09147d4..81bf40320100 100644 --- a/ext/sysvshm/sysvshm.c +++ b/ext/sysvshm/sysvshm.c @@ -90,6 +90,7 @@ ZEND_GET_MODULE(sysvshm) /* TODO: Make this thread-safe. */ sysvshm_module php_sysvshm; +static bool php_check_shm_head(const sysvshm_chunk_head *ptr, zend_long shm_size); static int php_put_shm_data(sysvshm_chunk_head *ptr, zend_long key, const char *data, zend_long len); static zend_long php_check_shm_data(sysvshm_chunk_head *ptr, zend_long key); static int php_remove_shm_data(sysvshm_chunk_head *ptr, zend_long shm_varpos); @@ -196,6 +197,10 @@ PHP_FUNCTION(shm_attach) chunk_ptr->end = chunk_ptr->start; chunk_ptr->total = shm_size; chunk_ptr->free = shm_size-chunk_ptr->end; + } else if (!php_check_shm_head(chunk_ptr, shm_size)) { + php_error_docref(NULL, E_WARNING, "Failed for key 0x" ZEND_XLONG_FMT ": segment header is corrupted", shm_key); + shmdt(shm_ptr); + RETURN_FALSE; } object_init_ex(return_value, sysvshm_ce); @@ -309,6 +314,8 @@ PHP_FUNCTION(shm_get_var) sysvshm_shm *shm_list_ptr; char *shm_data; zend_long shm_varpos; + zend_long shm_avail; + zend_long shm_len; sysvshm_chunk *shm_var; php_unserialize_data_t var_hash; @@ -331,10 +338,16 @@ PHP_FUNCTION(shm_get_var) RETURN_FALSE; } shm_var = (sysvshm_chunk*) ((char *)shm_list_ptr->ptr + shm_varpos); + shm_avail = shm_list_ptr->ptr->end - shm_varpos - (zend_long) sizeof(sysvshm_chunk); + if (shm_var->length < 0 || shm_var->length > shm_avail) { + php_error_docref(NULL, E_WARNING, "Variable data in shared memory is corrupted"); + RETURN_FALSE; + } + shm_len = shm_var->length; shm_data = &shm_var->mem; PHP_VAR_UNSERIALIZE_INIT(var_hash); - int res = php_var_unserialize(return_value, (const unsigned char **) &shm_data, (unsigned char *) shm_data + shm_var->length, &var_hash); + int res = php_var_unserialize(return_value, (const unsigned char **) &shm_data, (unsigned char *) shm_data + shm_len, &var_hash); PHP_VAR_UNSERIALIZE_DESTROY(var_hash); if (res != 1) { php_error_docref(NULL, E_WARNING, "Variable data in shared memory is corrupted"); @@ -388,11 +401,33 @@ PHP_FUNCTION(shm_remove_var) php_error_docref(NULL, E_WARNING, "Variable key " ZEND_LONG_FMT " doesn't exist", shm_key); RETURN_FALSE; } - php_remove_shm_data((shm_list_ptr->ptr), shm_varpos); + if (php_remove_shm_data((shm_list_ptr->ptr), shm_varpos) < 0) { + php_error_docref(NULL, E_WARNING, "Variable data in shared memory is corrupted"); + RETURN_FALSE; + } RETURN_TRUE; } /* }}} */ +/* {{{ php_check_shm_head */ +static bool php_check_shm_head(const sysvshm_chunk_head *ptr, zend_long shm_size) +{ + if (ptr->total < (zend_long) sizeof(sysvshm_chunk_head) || ptr->total > shm_size) { + return false; + } + if (ptr->start < (zend_long) sizeof(sysvshm_chunk_head) || ptr->start > ptr->total) { + return false; + } + if (ptr->end < ptr->start || ptr->end > ptr->total) { + return false; + } + if (ptr->free < 0 || ptr->free > ptr->total - ptr->end) { + return false; + } + return true; +} +/* }}} */ + /* {{{ php_put_shm_data * inserts an ascii-string into shared memory */ 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) pos = ptr->start; for (;;) { - if (pos >= ptr->end) { + if (ptr->end - pos < (zend_long) sizeof(sysvshm_chunk)) { return -1; } 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) ZEND_ASSERT(ptr); chunk_ptr = (sysvshm_chunk *) ((char *) ptr + shm_varpos); + + if (chunk_ptr->next <= 0 || chunk_ptr->next > ptr->end - shm_varpos) { + return -1; + } + next_chunk_ptr = (sysvshm_chunk *) ((char *) ptr + shm_varpos + chunk_ptr->next); memcpy_len = ptr->end-shm_varpos - chunk_ptr->next; diff --git a/ext/sysvshm/tests/shm_attach_header_bounds.phpt b/ext/sysvshm/tests/shm_attach_header_bounds.phpt new file mode 100644 index 000000000000..bf3e0c450ed7 --- /dev/null +++ b/ext/sysvshm/tests/shm_attach_header_bounds.phpt @@ -0,0 +1,40 @@ +--TEST-- +sysvshm: shm_attach() must reject a segment whose header is out of bounds +--EXTENSIONS-- +sysvshm +shmop +--SKIPIF-- + +--FILE-- + +--EXPECTF-- +Warning: shm_attach(): Failed for key 0x%x: segment header is corrupted in %s on line %d +bool(false) +Done +--CLEAN-- + diff --git a/ext/sysvshm/tests/shm_get_var_chunk_length_bounds.phpt b/ext/sysvshm/tests/shm_get_var_chunk_length_bounds.phpt new file mode 100644 index 000000000000..67e686419757 --- /dev/null +++ b/ext/sysvshm/tests/shm_get_var_chunk_length_bounds.phpt @@ -0,0 +1,82 @@ +--TEST-- +shm_get_var() must not trust the chunk length stored in a hostile segment for unserialize() +--EXTENSIONS-- +sysvshm +ffi +--INI-- +ffi.enable=1 +--SKIPIF-- + +--FILE-- +shmget($key, 4096, 0666 | 01000); + if ($id < 0) { + echo "shm setup failed\n"; + return; + } + $p = $ffi->shmat($id, NULL, 0); + if ($p == $ffi->cast('char*', -1)) { + echo "shmat failed\n"; + return; + } + FFI::memset($p, 0, 4096); + $head = $ffi->cast('head_t*', $p); + FFI::memcpy($head->magic, "PHP_SM", 6); + $head->start = 40; + $head->end = 4096; + $head->free_ = 0; + $head->total = 4096; + $chunk = $ffi->cast('chunk_t*', $ffi->cast('char*', $p) + 40); + $chunk->key = 1; + $chunk->length = $len; + $chunk->next = 4096 - 40; + FFI::memcpy($chunk->mem, "i:42;", 5); +} + +$key1 = 0x5A5A0E01; +$key2 = 0x5A5A0E02; + +$old = @shm_attach($key1); +if ($old !== false) { + shm_remove($old); +} +$old = @shm_attach($key2); +if ($old !== false) { + shm_remove($old); +} +craft_hostile_segment($key1, PHP_INT_MAX); + +$shm = shm_attach($key1, 4096); +var_dump(shm_has_var($shm, 1)); +var_dump(shm_get_var($shm, 1)); +shm_remove($shm); + +$shm2 = shm_attach($key2, 4096); +shm_put_var($shm2, 1, 42); +var_dump(shm_get_var($shm2, 1)); +shm_remove($shm2); + +echo "Done\n"; +?> +--EXPECTF-- +bool(true) + +Warning: shm_get_var(): Variable data in shared memory is corrupted in %s on line %d +bool(false) +int(42) +Done diff --git a/ext/sysvshm/tests/shm_has_var_start_bounds.phpt b/ext/sysvshm/tests/shm_has_var_start_bounds.phpt new file mode 100644 index 000000000000..bd520fe34fcc --- /dev/null +++ b/ext/sysvshm/tests/shm_has_var_start_bounds.phpt @@ -0,0 +1,37 @@ +--TEST-- +sysvshm: shm_has_var() must reject a segment whose start offset is out of bounds +--EXTENSIONS-- +sysvshm +shmop +--SKIPIF-- + +--FILE-- + +--EXPECTF-- +Warning: shm_attach(): Failed for key 0x%x: segment header is corrupted in %s on line %d +bool(false) +Done +--CLEAN-- + diff --git a/ext/sysvshm/tests/shm_remove_var_end_bounds.phpt b/ext/sysvshm/tests/shm_remove_var_end_bounds.phpt new file mode 100644 index 000000000000..42da21fe6274 --- /dev/null +++ b/ext/sysvshm/tests/shm_remove_var_end_bounds.phpt @@ -0,0 +1,38 @@ +--TEST-- +sysvshm: shm_remove_var() must not memmove past a segment with a corrupt end +--EXTENSIONS-- +sysvshm +shmop +--SKIPIF-- + +--FILE-- + +--EXPECTF-- +Warning: shm_attach(): Failed for key 0x%x: segment header is corrupted in %s on line %d +bool(false) +Done +--CLEAN-- +