Validate stored length before decompressing

package.xml

@@ -97,6 +97,9 @@ http://pear.php.net/dtd/package-2.0.xsd">
     <file role='test' name='clone.phpt'/>
     <file role='test' name='compression_conditions.phpt'/>
     <file role='test' name='compression_types.phpt'/>
+    <file role='test' name='compression_length.inc'/>
+    <file role='test' name='compression_length_header.phpt'/>
+    <file role='test' name='compression_length_header_zstd.phpt'/>
     <file role='test' name='conf_persist.phpt'/>
     <file role='test' name='construct.phpt'/>
     <file role='test' name='construct_persistent.phpt'/>

php_memcached.c

@@ -3713,12 +3713,26 @@ memcached_return s_server_cursor_version_cb(const memcached_st *ptr, php_memcach
 }
 
 
+/* Most uncompressed bytes a single compressed byte can produce, so a stored
+ * length that no input of this size could reach is rejected before it is
+ * allocated:
+ * - zlib: a 258-byte match costs at least two bits, a limit of 1032:1
+ *   (https://zlib.net/zlib_tech.html).
+ * - FastLZ: a level 2 match grows by at most 255 bytes per input byte, and a
+ *   level 1 match yields at most 264 bytes from three (fastlz/fastlz.c).
+ * - zstd: an RLE block repeats one byte up to 128 KiB behind a three-byte
+ *   block header (RFC 8878, section 3.1.1.2). */
+#define MEMC_ZLIB_MAX_RATIO   1032
+#define MEMC_FASTLZ_MAX_RATIO 255
+#define MEMC_ZSTD_MAX_RATIO   ((128 * 1024) / 4)
+
 static
 zend_string *s_decompress_value (const char *payload, size_t payload_len, uint32_t flags)
 {
 	zend_string *buffer;
 
 	uint32_t stored_length;
+	uint32_t max_ratio;
 	unsigned long length;
 	zend_bool decompress_status = 0;
 	zend_bool is_fastlz = 0, is_zlib = 0, is_zstd = 0;
@@ -3748,27 +3762,53 @@ zend_string *s_decompress_value (const char *payload, size_t payload_len, uint32
 	payload     += sizeof (uint32_t);
 	payload_len -= sizeof (uint32_t);
 
-	buffer = zend_string_alloc (stored_length, 0);
+	if (is_zstd) {
+		max_ratio = MEMC_ZSTD_MAX_RATIO;
+	} else if (is_fastlz) {
+		max_ratio = MEMC_FASTLZ_MAX_RATIO;
+	} else {
+		max_ratio = MEMC_ZLIB_MAX_RATIO;
+	}
+
+	if ((uint64_t) stored_length > (uint64_t) payload_len * max_ratio) {
+		php_error_docref(NULL, E_WARNING, "could not decompress value: stored length does not match the compressed data");
+		return NULL;
+	}
 
 #ifdef HAVE_ZSTD_H
 	if (is_zstd) {
-		length = ZSTD_getFrameContentSize(payload, payload_len);
-		if (length == ZSTD_CONTENTSIZE_ERROR) {
+		unsigned long long frame_length = ZSTD_getFrameContentSize(payload, payload_len);
+		if (frame_length == ZSTD_CONTENTSIZE_ERROR) {
 			php_error_docref(NULL, E_WARNING, "value was not compressed by zstd");
-			zend_string_release (buffer);
 			return NULL;
-		} else if (length == ZSTD_CONTENTSIZE_UNKNOWN) {
+		} else if (frame_length == ZSTD_CONTENTSIZE_UNKNOWN) {
 			php_error_docref(NULL, E_WARNING, "zstd streaming decompression not supported");
-			zend_string_release (buffer);
+			return NULL;
+		} else if (frame_length != stored_length) {
+			php_error_docref(NULL, E_WARNING, "could not decompress value: stored length does not match the compressed data");
 			return NULL;
 		}
-		decompress_status = !ZSTD_isError(ZSTD_decompress(&buffer->val, buffer->len, payload, payload_len));
+	}
+#endif
+
+	buffer = zend_string_alloc (stored_length, 0);
 
+#ifdef HAVE_ZSTD_H
+	if (is_zstd) {
+		size_t zstd_length = ZSTD_decompress(&buffer->val, buffer->len, payload, payload_len);
+
+		decompress_status = !ZSTD_isError(zstd_length);
+		if (decompress_status) {
+			buffer->len = zstd_length;
+		}
 	}
 	else
 #endif
 	if (is_fastlz) {
 		decompress_status = ((length = fastlz_decompress(payload, payload_len, &buffer->val, buffer->len)) > 0);
+		if (decompress_status) {
+			buffer->len = length;
+		}
 	}
 	else if (is_zlib) {
 		unsigned long ds = buffer->len;
@@ -3777,14 +3817,13 @@ zend_string *s_decompress_value (const char *payload, size_t payload_len, uint32
 		buffer->len = ds;
 	}
 
-	ZSTR_VAL(buffer)[stored_length] = '\0';
-
 	if (!decompress_status) {
 		php_error_docref(NULL, E_WARNING, "could not decompress value");
 		zend_string_release (buffer);
 		return NULL;
 	}
 
+	ZSTR_VAL(buffer)[ZSTR_LEN(buffer)] = '\0';
 	zend_string_forget_hash_val(buffer);
 	return buffer;
 }

tests/compression_length.inc

@@ -0,0 +1,60 @@
+<?php
+
+function memc_raw_command($command)
+{
+	$fp = fsockopen(MEMC_SERVER_HOST, MEMC_SERVER_PORT, $errno, $errstr, 2);
+	fwrite($fp, $command);
+	$reply = '';
+	while (!feof($fp)) {
+		$line = fgets($fp);
+		$reply .= $line;
+		if ($line === "END\r\n" || $line === "STORED\r\n" || strncmp($line, "ERROR", 5) === 0) {
+			break;
+		}
+	}
+	fclose($fp);
+	return $reply;
+}
+
+/* Returns [flags, data] as stored, without client-side decoding. */
+function memc_raw_get($key)
+{
+	$reply = memc_raw_command("get $key\r\n");
+	$header_end = strpos($reply, "\r\n");
+	list(, , $flags, $len) = explode(' ', substr($reply, 0, $header_end));
+	return array((int) $flags, substr($reply, $header_end + 2, (int) $len));
+}
+
+function memc_raw_set($key, $flags, $data)
+{
+	return trim(memc_raw_command("set $key $flags 0 " . strlen($data) . "\r\n" . $data . "\r\n"));
+}
+
+/* Stores $value compressed with $type, then rewrites the 4-byte uncompressed
+ * length header that precedes the compressed data, and reads it back. */
+function memc_compressed_length_test($m, $name, $type, $value)
+{
+	$m->setOption(Memcached::OPT_COMPRESSION, true);
+	$m->setOption(Memcached::OPT_COMPRESSION_TYPE, $type);
+	$m->set('length_test', $value);
+	list($flags, $data) = memc_raw_get('length_test');
+	$real = unpack('V', substr($data, 0, 4))[1];
+	$body = substr($data, 4);
+
+	$cases = array(
+		'unchanged' => $real,
+		'larger by 4096' => $real + 4096,
+		'smaller by 1' => $real - 1,
+		'1 GiB' => 1 << 30,
+		'4 GiB - 1' => 0xFFFFFFFF,
+	);
+	foreach ($cases as $label => $length) {
+		memc_raw_set('length_test', $flags, pack('V', $length) . $body);
+		error_clear_last();
+		$result = @$m->get('length_test');
+		$error = error_get_last();
+		printf("%s %s: %s%s\n", $name, $label,
+			$result === $value ? 'original value' : var_export($result, true),
+			$error ? ' (' . preg_replace('/^Memcached::get\(\): /', '', $error['message']) . ')' : '');
+	}
+}

tests/compression_length_header.phpt

@@ -0,0 +1,28 @@
+--TEST--
+Compressed item with inconsistent length header (fastlz, zlib)
+--SKIPIF--
+<?php include "skipif.inc";?>
+--INI--
+memory_limit=128M
+--FILE--
+<?php
+include dirname(__FILE__) . '/config.inc';
+include dirname(__FILE__) . '/compression_length.inc';
+
+$m = memc_get_instance(array(Memcached::OPT_SERIALIZER => Memcached::SERIALIZER_PHP));
+$value = str_repeat('abcdefgh', 500);
+
+memc_compressed_length_test($m, 'fastlz', Memcached::COMPRESSION_FASTLZ, $value);
+memc_compressed_length_test($m, 'zlib', Memcached::COMPRESSION_ZLIB, $value);
+?>
+--EXPECT--
+fastlz unchanged: original value
+fastlz larger by 4096: original value
+fastlz smaller by 1: false (could not decompress value)
+fastlz 1 GiB: false (could not decompress value: stored length does not match the compressed data)
+fastlz 4 GiB - 1: false (could not decompress value: stored length does not match the compressed data)
+zlib unchanged: original value
+zlib larger by 4096: original value
+zlib smaller by 1: false (could not decompress value)
+zlib 1 GiB: false (could not decompress value: stored length does not match the compressed data)
+zlib 4 GiB - 1: false (could not decompress value: stored length does not match the compressed data)

tests/compression_length_header_zstd.phpt

@@ -0,0 +1,25 @@
+--TEST--
+Compressed item with inconsistent length header (zstd)
+--SKIPIF--
+<?php
+include "skipif.inc";
+if (!Memcached::HAVE_ZSTD) die("skip zstd support not enabled\n");
+?>
+--INI--
+memory_limit=128M
+--FILE--
+<?php
+include dirname(__FILE__) . '/config.inc';
+include dirname(__FILE__) . '/compression_length.inc';
+
+$m = memc_get_instance(array(Memcached::OPT_SERIALIZER => Memcached::SERIALIZER_PHP));
+$value = str_repeat('abcdefgh', 500);
+
+memc_compressed_length_test($m, 'zstd', Memcached::COMPRESSION_ZSTD, $value);
+?>
+--EXPECT--
+zstd unchanged: original value
+zstd larger by 4096: false (could not decompress value: stored length does not match the compressed data)
+zstd smaller by 1: false (could not decompress value: stored length does not match the compressed data)
+zstd 1 GiB: false (could not decompress value: stored length does not match the compressed data)
+zstd 4 GiB - 1: false (could not decompress value: stored length does not match the compressed data)