fix: Don't blindly return LZ4 header length strings

Previously we were only checking if `LZ4_decompress_safe` was returning
> 0 but then blindly returning to the user whatever length the header
specified.

This fix does two things:

* Short circuits on negative length headers
* Fails the decompression if the decompressed length does not match.
This commit is contained in:
michael-grunder
2026-05-21 08:17:46 -07:00
committed by Michael Grunder
parent fbf5affa14
commit 2bf673c64f
2 changed files with 44 additions and 4 deletions
+7 -3
View File
@@ -4179,7 +4179,7 @@ redis_uncompress(RedisSock *redis_sock, char **dst, size_t *dstlen, const char *
#ifdef HAVE_REDIS_LZ4
{
char *data;
int datalen;
int datalen, res;
uint8_t lz4crc;
/* We must have at least enough bytes for our header, and can't have more than
@@ -4197,15 +4197,19 @@ redis_uncompress(RedisSock *redis_sock, char **dst, size_t *dstlen, const char *
memcpy(&datalen, copy, sizeof(int));
copy += sizeof(int); copylen -= sizeof(int);
if (datalen <= 0)
break;
/* Make sure our CRC matches (TODO: Maybe issue a docref error?) */
if (crc8((unsigned char*)&datalen, sizeof(datalen)) != lz4crc)
break;
/* Finally attempt decompression */
data = emalloc(datalen);
if (LZ4_decompress_safe(copy, data, copylen, datalen) > 0) {
res = LZ4_decompress_safe(copy, data, copylen, datalen);
if (res == datalen) {
*dst = data;
*dstlen = datalen;
*dstlen = res;
return 1;
}
+37 -1
View File
@@ -5576,15 +5576,51 @@ class Redis_Test extends TestSuite {
$this->checkCompression(Redis::COMPRESSION_ZSTD, 9);
}
public function testCompressionLZ4() {
if ( ! defined('Redis::COMPRESSION_LZ4'))
$this->markTestSkipped();
$this->redis->setOption(Redis::OPT_COMPRESSION, Redis::COMPRESSION_LZ4);
$payload = $this->lz4PayloadWithDeclaredLength('LEAK_ME!', 4096);
$this->assertThrowsMatch($payload, function ($payload) {
$this->redis->_uncompress($payload);
}, '/Invalid compressed data or uncompression error/');
$this->checkCompression(Redis::COMPRESSION_LZ4, 0);
$this->checkCompression(Redis::COMPRESSION_LZ4, 9);
}
private function lz4PayloadWithDeclaredLength($value, $declared_len) {
$len = strlen($value);
$lz4 = chr(min($len, 15) << 4);
if ($len >= 15) {
$extra = $len - 15;
while ($extra >= 255) {
$lz4 .= "\xff";
$extra -= 255;
}
$lz4 .= chr($extra);
}
$declared = pack('V', $declared_len);
return chr($this->crc8($declared)) . $declared . $lz4 . $value;
}
private function crc8($value) {
$crc = 0xff;
for ($i = 0; $i < strlen($value); $i++) {
$crc ^= ord($value[$i]);
for ($j = 0; $j < 8; $j++) {
$crc = $crc & 0x80 ? (($crc << 1) ^ 0x31) & 0xff : ($crc << 1) & 0xff;
}
}
return $crc;
}
private function checkCompression($mode, $level) {
$set_cmp = $this->redis->setOption(Redis::OPT_COMPRESSION, $mode);
$this->assertTrue($set_cmp);