Skip to content

Commit

Permalink
Add limit parameter to decompressZlib
Browse files Browse the repository at this point in the history
This can prevent untrusted data, such as sent over the network,
from consuming all memory with a specially crafted payload.
  • Loading branch information
bendeutsch authored and sfan5 committed Feb 1, 2020
1 parent 1116918 commit 2b3490d
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 4 deletions.
20 changes: 17 additions & 3 deletions src/serialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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)
{
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/serialization.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
63 changes: 63 additions & 0 deletions src/unittest/test_compression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ class TestCompression : public TestBase {
void testRLECompression();
void testZlibCompression();
void testZlibLargeData();
void testZlibLimit();
void _testZlibLimit(u32 size, u32 limit);
};

static TestCompression g_test_instance;
Expand All @@ -46,6 +48,7 @@ void TestCompression::runTests(IGameDef *gamedef)
TEST(testRLECompression);
TEST(testZlibCompression);
TEST(testZlibLargeData);
TEST(testZlibLimit);
}

////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -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]);
}
}

0 comments on commit 2b3490d

Please sign in to comment.