If is podofo exception thrown between "new" and "delete" then this memory
will be leaked. Lines 638-663:
```
char *buffer = new char[bufferSize];
if( !buffer )
{
TIFFClose(hInTiffHandle);
PODOFO_RAISE_ERROR( ePdfError_OutOfMemory );
}
for(row = 0; row < height; row++)
{
if(TIFFReadScanline(hInTiffHandle, &buffer[row * scanlineSize],
row) == (-1))
{
TIFFClose(hInTiffHandle);
PODOFO_RAISE_ERROR( ePdfError_UnsupportedImageFormat );
}
}
...
delete[] buffer;
```
Notice that "buffer" will never be NULL because in case of failure "new"
will throw "std::bad_alloc" so "if" condition on second line is always
false.
It is not so hard to achieve this leak using invalid tiff file:
```
int main()
{
PdfMemDocument doc;
PdfImage image(&doc);
std::vector<unsigned char> corrupt_tiff = {
0x4D, 0x4D, 0x00, 0x2A,
0x00, 0x00, 0x00, 0x08,
0x00, 0x05,
0x01, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x01, 0x01, 0x00, 0x00,
0x00,
0x01, 0x01, 0x00, 0x03, 0x00, 0x00, 0x00, 0x01, 0x01, 0x00, 0x00,
0x00,
0x01, 0x06, 0x00, 0x03, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
0x00,
0x01, 0x11, 0x00, 0x03, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
0x00,
0x01, 0x17, 0x00, 0x03, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
0x00,
0x00, 0x00, 0x00, 0x00
};
try
{
image.LoadFromTiffData(corrupt_tiff.data(), corrupt_tiff.size());
}
catch(PdfError &e)
{
e.PrintErrorMsg();
}
return 0;
}
```
Valgrind:
```
==8384== HEAP SUMMARY:
==8384== in use at exit: 8,192 bytes in 1 blocks
==8384== total heap usage: 1,464 allocs, 1,463 frees, 207,921 bytes
allocated
==8384==
==8384== 8,192 bytes in 1 blocks are definitely lost in loss record 1 of 1
==8384== at 0x4C3089F: operator new[](unsigned long) (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8384== by 0x215545: PoDoFo::PdfImage::LoadFromTiffHandle(void*)
==8384== by 0x215EEC: PoDoFo::PdfImage::LoadFromTiffData(unsigned char
const*, long)
==8384== by 0x12C9EB: main
==8384==
==8384== LEAK SUMMARY:
==8384== definitely lost: 8,192 bytes in 1 blocks
==8384== indirectly lost: 0 bytes in 0 blocks
==8384== possibly lost: 0 bytes in 0 blocks
==8384== still reachable: 0 bytes in 0 blocks
==8384== suppressed: 0 bytes in 0 blocks
```
8192 bytes is allocation from line with "new char[bufferSize];".
Another leak is between lines 604 and 626:
```
char *datap = new char[numColors*3];
for ( int clr = 0; clr < numColors; clr++ )
{
datap[3*clr+0] = rgbRed[clr]/257;
datap[3*clr+1] = rgbGreen[clr]/257;
datap[3*clr+2] = rgbBlue[clr]/257;
}
PdfMemoryInputStream stream( datap, numColors*3 );
// Create a colorspace object
PdfObject* pIdxObject =
this->GetObject()->GetOwner()->CreateObject();
pIdxObject->GetStream()->Set( &stream );
// Add the colorspace to our image
PdfArray array;
array.push_back( PdfName("Indexed") );
array.push_back( PdfName("DeviceRGB") );
array.push_back( static_cast<pdf_int64>(numColors-1) );
array.push_back( pIdxObject->Reference() );
this->GetObject()->GetDictionary().AddKey(
PdfName("ColorSpace"), array );
delete[] datap;
```
There is no guaranty that these things between will not throw some
exception. At least there is used "new" inside
"PdfVecObjects::CreateObject" which can throw "std::bad_alloc". And this is
why it is not very good to use "new" and "delete" in C++. It has its
appropriate uses but this is not one of them. In this use case is much
better to use "std::unique_ptr" (or auto_ptr) for one unique dynamic
instance or "std::vector" for dynamic array. In year 2005 we already had
them. In such case it will be freed also in case of exception.
Patch attached. There can be still leaked "hInTiffHandle" in case of memory
allocation failure as in original code. This would need bigger changes.
image_load_tiff_leaks.patch
Description: Binary data
_______________________________________________ Podofo-users mailing list Podofo-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/podofo-users
