Fix memory leak loading malformed `SawyerChunk` (#21508)

* Fix memory leak loading malformed `SawyerChunk`

A temporary buffer was not free'd after failing to parse in
`SawyerChunkReader::ReadChunkTrack`. Fix this following the pattern used
in `SawyerChunkReader::ReadChunk` by wrapping the relevant code in a
`try` block with `FreeLargeTempBuffer` called when an exception is
caught.

* Use unique_ptr

* Remove `AllocateLargeTempBuffer`

---------

Co-authored-by: Michał Janiszewski <janisozaur@gmail.com>
This commit is contained in:
John Kastner 2024-04-01 18:40:14 -04:00 committed by GitHub
parent a94e6c54d8
commit 6ea091841f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 18 additions and 66 deletions

View File

@ -12,14 +12,9 @@
#include "../core/Memory.hpp"
#include "SawyerChunkReader.h"
SawyerChunk::SawyerChunk(SAWYER_ENCODING encoding, void* data, size_t length)
SawyerChunk::SawyerChunk(SAWYER_ENCODING encoding, std::unique_ptr<uint8_t[]> data, size_t length)
{
_encoding = encoding;
_data = data;
_data = std::move(data);
_length = length;
}
SawyerChunk::~SawyerChunk()
{
SawyerChunkReader::FreeChunk(_data);
}

View File

@ -11,6 +11,8 @@
#include "../common.h"
#include <memory>
/**
* The type of encoding / compression for a sawyer encoded chunk.
*/
@ -28,14 +30,14 @@ enum class SAWYER_ENCODING : uint8_t
class SawyerChunk final
{
private:
void* _data = nullptr;
std::unique_ptr<uint8_t[]> _data;
size_t _length = 0;
SAWYER_ENCODING _encoding = SAWYER_ENCODING::NONE;
public:
const void* GetData() const
{
return _data;
return _data.get();
}
size_t GetLength() const
{
@ -46,6 +48,5 @@ public:
return _encoding;
}
SawyerChunk(SAWYER_ENCODING encoding, void* data, size_t length);
~SawyerChunk();
SawyerChunk(SAWYER_ENCODING encoding, std::unique_ptr<uint8_t[]> data, size_t length);
};

View File

@ -71,22 +71,15 @@ std::shared_ptr<SawyerChunk> SawyerChunkReader::ReadChunk()
throw SawyerChunkException(EXCEPTION_MSG_CORRUPT_CHUNK_SIZE);
}
auto buffer = static_cast<uint8_t*>(AllocateLargeTempBuffer());
try
auto buffer = std::make_unique<uint8_t[]>(MAX_UNCOMPRESSED_CHUNK_SIZE);
size_t uncompressedLength = DecodeChunk(
buffer.get(), MAX_UNCOMPRESSED_CHUNK_SIZE, compressedData.get(), header);
if (uncompressedLength == 0)
{
size_t uncompressedLength = DecodeChunk(buffer, MAX_UNCOMPRESSED_CHUNK_SIZE, compressedData.get(), header);
if (uncompressedLength == 0)
{
throw SawyerChunkException(EXCEPTION_MSG_ZERO_SIZED_CHUNK);
}
return std::make_shared<SawyerChunk>(
static_cast<SAWYER_ENCODING>(header.encoding), buffer, uncompressedLength);
}
catch (const std::exception&)
{
FreeLargeTempBuffer(buffer);
throw;
throw SawyerChunkException(EXCEPTION_MSG_ZERO_SIZED_CHUNK);
}
return std::make_shared<SawyerChunk>(
static_cast<SAWYER_ENCODING>(header.encoding), std::move(buffer), uncompressedLength);
}
default:
throw SawyerChunkException(EXCEPTION_MSG_INVALID_CHUNK_ENCODING);
@ -119,14 +112,14 @@ std::shared_ptr<SawyerChunk> SawyerChunkReader::ReadChunkTrack()
throw SawyerChunkException(EXCEPTION_MSG_CORRUPT_CHUNK_SIZE);
}
auto buffer = static_cast<uint8_t*>(AllocateLargeTempBuffer());
auto buffer = std::make_unique<uint8_t[]>(MAX_UNCOMPRESSED_CHUNK_SIZE);
SawyerCodingChunkHeader header{ CHUNK_ENCODING_RLE, compressedDataLength };
size_t uncompressedLength = DecodeChunk(buffer, MAX_UNCOMPRESSED_CHUNK_SIZE, compressedData.get(), header);
size_t uncompressedLength = DecodeChunk(buffer.get(), MAX_UNCOMPRESSED_CHUNK_SIZE, compressedData.get(), header);
if (uncompressedLength == 0)
{
throw SawyerChunkException(EXCEPTION_MSG_ZERO_SIZED_CHUNK);
}
return std::make_shared<SawyerChunk>(SAWYER_ENCODING::RLE, buffer, uncompressedLength);
return std::make_shared<SawyerChunk>(SAWYER_ENCODING::RLE, std::move(buffer), uncompressedLength);
}
catch (const std::exception&)
{
@ -157,11 +150,6 @@ void SawyerChunkReader::ReadChunk(void* dst, size_t length)
}
}
void SawyerChunkReader::FreeChunk(void* data)
{
FreeLargeTempBuffer(data);
}
size_t SawyerChunkReader::DecodeChunk(void* dst, size_t dstCapacity, const void* src, const SawyerCodingChunkHeader& header)
{
size_t resultLength;
@ -192,8 +180,7 @@ size_t SawyerChunkReader::DecodeChunk(void* dst, size_t dstCapacity, const void*
size_t SawyerChunkReader::DecodeChunkRLERepeat(void* dst, size_t dstCapacity, const void* src, size_t srcLength)
{
auto immBuffer = std::unique_ptr<uint8_t, decltype(&FreeLargeTempBuffer)>(
static_cast<uint8_t*>(AllocateLargeTempBuffer()), &FreeLargeTempBuffer);
auto immBuffer = std::make_unique<uint8_t[]>(MAX_UNCOMPRESSED_CHUNK_SIZE);
auto immLength = DecodeChunkRLE(immBuffer.get(), MAX_UNCOMPRESSED_CHUNK_SIZE, src, srcLength);
auto size = DecodeChunkRepeat(dst, dstCapacity, immBuffer.get(), immLength);
return size;
@ -301,26 +288,3 @@ size_t SawyerChunkReader::DecodeChunkRotate(void* dst, size_t dstCapacity, const
}
return srcLength;
}
void* SawyerChunkReader::AllocateLargeTempBuffer()
{
#ifdef __USE_HEAP_ALLOC__
auto buffer = HeapAlloc(GetProcessHeap(), 0, MAX_UNCOMPRESSED_CHUNK_SIZE);
#else
auto buffer = std::malloc(MAX_UNCOMPRESSED_CHUNK_SIZE);
#endif
if (buffer == nullptr)
{
throw std::runtime_error("Unable to allocate large temporary buffer.");
}
return buffer;
}
void SawyerChunkReader::FreeLargeTempBuffer(void* buffer)
{
#ifdef __USE_HEAP_ALLOC__
HeapFree(GetProcessHeap(), 0, buffer);
#else
std::free(buffer);
#endif
}

View File

@ -85,18 +85,10 @@ public:
return result;
}
/**
* Frees the chunk data, to be used when destructing SawyerChunks
*/
static void FreeChunk(void* data);
private:
static size_t DecodeChunk(void* dst, size_t dstCapacity, const void* src, const SawyerCodingChunkHeader& header);
static size_t DecodeChunkRLERepeat(void* dst, size_t dstCapacity, const void* src, size_t srcLength);
static size_t DecodeChunkRLE(void* dst, size_t dstCapacity, const void* src, size_t srcLength);
static size_t DecodeChunkRepeat(void* dst, size_t dstCapacity, const void* src, size_t srcLength);
static size_t DecodeChunkRotate(void* dst, size_t dstCapacity, const void* src, size_t srcLength);
static void* AllocateLargeTempBuffer();
static void FreeLargeTempBuffer(void* buffer);
};