From dc1b37845411e5ee0765bf4e95e695f467dd8894 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Thu, 15 Oct 2026 18:27:30 -0400 Subject: [PATCH 1/3] Validate stored length before decompressing A compressed item starts with its uncompressed length, and the read path allocated that many bytes before decompressing. Reject a length larger than the compressed data can expand to (1032:1 for zlib, 255:1 for FastLZ, 32768:1 for zstd), and for zstd require it to match the frame's content size. A rejected length fails like any other decompression error: a warning and a false result. --- php_memcached.c | 46 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/php_memcached.c b/php_memcached.c index 47ed2d33..166a28a1 100644 --- a/php_memcached.c +++ b/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,22 +3762,40 @@ 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) { + decompress_status = !ZSTD_isError(ZSTD_decompress(&buffer->val, buffer->len, payload, payload_len)); } else #endif From ad610a55d64d991422ea188e8923f425f1ba0092 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Thu, 15 Oct 2026 18:27:31 -0400 Subject: [PATCH 2/3] Use the decompressed size as the value length FastLZ and zstd results kept the stored length as the string length, so a stored length larger than the decompressed output left uninitialized bytes at the end of the value. Set the length to what the decompressor returned, as the zlib path already does, and terminate the string there. --- php_memcached.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/php_memcached.c b/php_memcached.c index 166a28a1..af89f64a 100644 --- a/php_memcached.c +++ b/php_memcached.c @@ -3795,12 +3795,20 @@ zend_string *s_decompress_value (const char *payload, size_t payload_len, uint32 #ifdef HAVE_ZSTD_H if (is_zstd) { - decompress_status = !ZSTD_isError(ZSTD_decompress(&buffer->val, buffer->len, payload, payload_len)); + 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; @@ -3809,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; } From af872322d39940837663721da109492ed56cc63c Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Thu, 15 Oct 2026 18:27:31 -0400 Subject: [PATCH 3/3] tests: Cover compressed items with an inconsistent length header --- package.xml | 3 ++ tests/compression_length.inc | 60 +++++++++++++++++++++++ tests/compression_length_header.phpt | 28 +++++++++++ tests/compression_length_header_zstd.phpt | 25 ++++++++++ 4 files changed, 116 insertions(+) create mode 100644 tests/compression_length.inc create mode 100644 tests/compression_length_header.phpt create mode 100644 tests/compression_length_header_zstd.phpt diff --git a/package.xml b/package.xml index 524a51e1..fadf9352 100644 --- a/package.xml +++ b/package.xml @@ -97,6 +97,9 @@ http://pear.php.net/dtd/package-2.0.xsd"> + + + diff --git a/tests/compression_length.inc b/tests/compression_length.inc new file mode 100644 index 00000000..4090f9ca --- /dev/null +++ b/tests/compression_length.inc @@ -0,0 +1,60 @@ +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']) . ')' : ''); + } +} diff --git a/tests/compression_length_header.phpt b/tests/compression_length_header.phpt new file mode 100644 index 00000000..fa9e5d84 --- /dev/null +++ b/tests/compression_length_header.phpt @@ -0,0 +1,28 @@ +--TEST-- +Compressed item with inconsistent length header (fastlz, zlib) +--SKIPIF-- + +--INI-- +memory_limit=128M +--FILE-- + 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) diff --git a/tests/compression_length_header_zstd.phpt b/tests/compression_length_header_zstd.phpt new file mode 100644 index 00000000..b0234f10 --- /dev/null +++ b/tests/compression_length_header_zstd.phpt @@ -0,0 +1,25 @@ +--TEST-- +Compressed item with inconsistent length header (zstd) +--SKIPIF-- + +--INI-- +memory_limit=128M +--FILE-- + 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)