[sysvshm] Clamp shm_get_var chunk length to segment bounds

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)

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;

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--
+<?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); }
+?>

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--
+<?php
+if (PHP_OS_FAMILY !== 'Linux') {
+    die('skip shmget is not available to FFI on this OS');
+}
+if (PHP_INT_SIZE !== 8) {
+    die('skip FFI chunk layout assumes 64-bit zend_long');
+}
+?>
+--FILE--
+<?php
+
+function craft_hostile_segment(int $key, int $len): void {
+    $ffi = FFI::cdef("
+        typedef struct { char magic[6]; long start; long end; long free_; long total; } head_t;
+        typedef struct { long key; long length; long next; char mem[16]; } chunk_t;
+        int shmget(int, int, int);
+        void *shmat(int, const void *, int);
+    ");
+    $id = $ffi->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

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--
+<?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); }
+?>

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--
+<?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); }
+?>