Fix LZF decompression logic. (#2065)

michael-grunder · yatsukhnenko · commit d938a4262189 · 2022-02-01T20:05:36.000+02:00
* Fix LZF decompression logic.

Rework how we decompress LZF data.  Previously it was possible to
encounter a double-free, if the error was not E2BIG.

* .
diff --git a/library.c b/library.c
@@ -2941,27 +2941,22 @@ redis_uncompress(RedisSock *redis_sock, char **dst, size_t *dstlen, const char *
         case REDIS_COMPRESSION_LZF:
 #ifdef HAVE_REDIS_LZF
             {
-                char *data;
-                int i;
+                char *data = NULL;
                 uint32_t res;
+                int i;
 
                 if (len == 0)
                     break;
 
-                /* start from two-times bigger buffer and
-                 * increase it exponentially  if needed */
+                /* Grow our buffer until we succeed or get a non E2BIG error */
                 errno = E2BIG;
                 for (i = 2; errno == E2BIG; i *= 2) {
-                    data = emalloc(i * len);
-                    if ((res = lzf_decompress(src, len, data, i * len)) == 0) {
-                        /* errno != E2BIG will brake for loop */
-                        efree(data);
-                        continue;
+                    data = erealloc(data, len * i);
+                    if ((res = lzf_decompress(src, len, data, len * i)) > 0) {
+                        *dst = data;
+                        *dstlen = res;
+                        return 1;
                     }
-
-                    *dst = data;
-                    *dstlen = res;
-                    return 1;
                 }
 
                 efree(data);
diff --git a/tests/RedisTest.php b/tests/RedisTest.php
@@ -4584,6 +4584,14 @@ public function testCompressionLZF()
         if (!defined('Redis::COMPRESSION_LZF')) {
             $this->markTestSkipped();
         }
+
+        /* Don't crash on improperly compressed LZF data */
+        $payload = 'not-actually-lzf-compressed';
+        $this->redis->set('badlzf', $payload);
+        $this->redis->setOption(Redis::OPT_COMPRESSION, Redis::COMPRESSION_LZF);
+        $this->assertEquals($payload, $this->redis->get('badlzf'));
+        $this->redis->setOption(Redis::OPT_COMPRESSION, Redis::COMPRESSION_NONE);
+
         $this->checkCompression(Redis::COMPRESSION_LZF, 0);
     }
 

Original file line number	Diff line number	Diff line change
`@@ -4584,6 +4584,14 @@ public function testCompressionLZF()`
`4584`	`4584`	`if (!defined('Redis::COMPRESSION_LZF')) {`
`4585`	`4585`	`$this->markTestSkipped();`
`4586`	`4586`	`}`
	`4587`	`+`
	`4588`	`+ /* Don't crash on improperly compressed LZF data */`
	`4589`	`+ $payload = 'not-actually-lzf-compressed';`
	`4590`	`+ $this->redis->set('badlzf', $payload);`
	`4591`	`+ $this->redis->setOption(Redis::OPT_COMPRESSION, Redis::COMPRESSION_LZF);`
	`4592`	`+ $this->assertEquals($payload, $this->redis->get('badlzf'));`
	`4593`	`+ $this->redis->setOption(Redis::OPT_COMPRESSION, Redis::COMPRESSION_NONE);`
	`4594`	`+`
`4587`	`4595`	`$this->checkCompression(Redis::COMPRESSION_LZF, 0);`
`4588`	`4596`	`}`
`4589`	`4597`