Add limit parameter to decompressZlib

This can prevent untrusted data, such as sent over the network,
from consuming all memory with a specially crafted payload.

diff --git a/src/serialization.cpp b/src/serialization.cpp
index 36ddb467cbbc5a7fa82fac1f51fff8e6acba225a..310604f54698a39569bfaf180518f7ee30d404a9 100644 (file)
--- a/src/serialization.cpp
+++ b/src/serialization.cpp
@@ -99,7 +99,7 @@ void compressZlib(const std::string &data, std::ostream &os, int level)
        compressZlib((u8*)data.c_str(), data.size(), os, level);
 }
 
-void decompressZlib(std::istream &is, std::ostream &os)
+void decompressZlib(std::istream &is, std::ostream &os, size_t limit)
 {
        z_stream z;
        const s32 bufsize = 16384;
@@ -108,6 +108,7 @@ void decompressZlib(std::istream &is, std::ostream &os)
        int status = 0;
        int ret;
        int bytes_read = 0;
+       int bytes_written = 0;
        int input_buffer_len = 0;
 
        z.zalloc = Z_NULL;
@@ -124,8 +125,20 @@ void decompressZlib(std::istream &is, std::ostream &os)
 
        for(;;)
        {
+               int output_size = bufsize;
                z.next_out = (Bytef*)output_buffer;
-               z.avail_out = bufsize;
+               z.avail_out = output_size;
+
+               if (limit) {
+                       int limit_remaining = limit - bytes_written;
+                       if (limit_remaining <= 0) {
+                               // we're aborting ahead of time - throw an error?
+                               break;
+                       }
+                       if (limit_remaining < output_size) {
+                               z.avail_out = output_size = limit_remaining;
+                       }
+               }
 
                if(z.avail_in == 0)
                {
@@ -153,10 +166,11 @@ void decompressZlib(std::istream &is, std::ostream &os)
                        zerr(status);
                        throw SerializationError("decompressZlib: inflate failed");
                }
-               int count = bufsize - z.avail_out;
+               int count = output_size - z.avail_out;
                //dstream<<"count="<<count<<std::endl;
                if(count)
                        os.write(output_buffer, count);
+               bytes_written += count;
                if(status == Z_STREAM_END)
                {
                        //dstream<<"Z_STREAM_END"<<std::endl;

diff --git a/src/serialization.h b/src/serialization.h
index 7f8b833825f86da4dfc0beebfc18724df3bd4457..f399983c49a09cb5b901ac439040453d9deca99f 100644 (file)
--- a/src/serialization.h
+++ b/src/serialization.h
@@ -87,7 +87,7 @@ inline bool ser_ver_supported(s32 v) {
 
 void compressZlib(const u8 *data, size_t data_size, std::ostream &os, int level = -1);
 void compressZlib(const std::string &data, std::ostream &os, int level = -1);
-void decompressZlib(std::istream &is, std::ostream &os);
+void decompressZlib(std::istream &is, std::ostream &os, size_t limit = 0);
 
 // These choose between zlib and a self-made one according to version
 void compress(const SharedBuffer<u8> &data, std::ostream &os, u8 version);

diff --git a/src/unittest/test_compression.cpp b/src/unittest/test_compression.cpp
index 7d0378131069950221c124d91e079feea6272a69..dfcadd4b27b7e809bf860bd8deb5c68f9b38260c 100644 (file)
--- a/src/unittest/test_compression.cpp
+++ b/src/unittest/test_compression.cpp
@@ -37,6 +37,8 @@ public:
        void testRLECompression();
        void testZlibCompression();
        void testZlibLargeData();
+       void testZlibLimit();
+       void _testZlibLimit(u32 size, u32 limit);
 };
 
 static TestCompression g_test_instance;
@@ -46,6 +48,7 @@ void TestCompression::runTests(IGameDef *gamedef)
        TEST(testRLECompression);
        TEST(testZlibCompression);
        TEST(testZlibLargeData);
+       TEST(testZlibLimit);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -170,3 +173,63 @@ void TestCompression::testZlibLargeData()
                                i, str_decompressed[i], i, data_in[i]);
        }
 }
+
+void TestCompression::testZlibLimit()
+{
+       // edge cases
+       _testZlibLimit(1024, 1023);
+       _testZlibLimit(1024, 1024);
+       _testZlibLimit(1024, 1025);
+
+       // test around buffer borders
+       u32 bufsize = 16384; // as in implementation
+       for (int s = -1; s <= 1; s++)
+       {
+               for (int l = -1; l <= 1; l++)
+               {
+                       _testZlibLimit(bufsize + s, bufsize + l);
+               }
+       }
+       // span multiple buffers
+       _testZlibLimit(35000, 22000);
+       _testZlibLimit(22000, 35000);
+}
+
+void TestCompression::_testZlibLimit(u32 size, u32 limit)
+{
+       infostream << "Test: Testing zlib wrappers with a decompression "
+               "memory limit of " << limit << std::endl;
+
+       infostream << "Test: Input size of compressZlib for limit is "
+               << size << std::endl;
+
+       // how much data we expect to get
+       u32 expected = size < limit ? size : limit;
+
+       // create recognizable data
+       std::string data_in;
+       data_in.resize(size);
+       for (u32 i = 0; i < size; i++)
+               data_in[i] = (u8)(i % 256);
+
+       std::ostringstream os_compressed(std::ios::binary);
+       compressZlib(data_in, os_compressed);
+       infostream << "Test: Output size of compressZlib for limit is "
+               << os_compressed.str().size()<<std::endl;
+
+       std::istringstream is_compressed(os_compressed.str(), std::ios::binary);
+       std::ostringstream os_decompressed(std::ios::binary);
+       decompressZlib(is_compressed, os_decompressed, limit);
+       infostream << "Test: Output size of decompressZlib with limit is "
+               << os_decompressed.str().size() << std::endl;
+
+       std::string str_decompressed = os_decompressed.str();
+       UASSERTEQ(size_t, str_decompressed.size(), expected);
+
+       for (u32 i = 0; i < size && i < str_decompressed.size(); i++) {
+               UTEST(str_decompressed[i] == data_in[i],
+                               "index out[%i]=%i differs from in[%i]=%i",
+                               i, str_decompressed[i], i, data_in[i]);
+       }
+}
+