vcl/source/filter/png/PngImageReader.cxx | 371 ++++++++++++++++---------------
1 file changed, 193 insertions(+), 178 deletions(-)
New commits:
commit 6f92f2cb118cc97e57c57e02bef491ecf39b1f4a
Author: Caolán McNamara <caolan.mcnam...@collabora.com>
AuthorDate: Thu Jul 31 21:01:51 2025 +0100
Commit: Caolán McNamara <caolan.mcnam...@collabora.com>
CommitDate: Fri Aug 1 20:19:27 2025 +0200
ofz#435489660 Direct-leak from vcl::PngImageReader::read
setjmp/longjmp skips std::vector dtor. This setjmp
has always been a pain, lets try replacing it
with a png_set_error_fn that does a c++ throw
Direct leak of 1384 byte(s) in 1 object(s) allocated from:
#0 0x567197b85dbd in operator new(unsigned long)
/src/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:86:3
#1 0x567197e42f98 in __libcpp_operator_new<unsigned long>
/usr/local/include/c++/v1/new:271:10
#2 0x567197e42f98 in __libcpp_allocate
/usr/local/include/c++/v1/new:295:10
#3 0x567197e42f98 in allocate
/usr/local/include/c++/v1/__memory/allocator.h:125:32
#4 0x567197e42f98 in __allocate_at_least<std::__1::allocator<unsigned
char> > /usr/local/include/c++/v1/__memory/allocate_at_least.h:55:19
#5 0x567197e42f98 in __vallocate /usr/local/include/c++/v1/vector:741:25
#6 0x567197e42f98 in std::__1::vector<unsigned char,
std::__1::allocator<unsigned char>>::vector(unsigned long, unsigned char
const&) /usr/local/include/c++/v1/vector:1144:5
#7 0x567197ef9095 in (anonymous namespace)::reader(SvStream&,
ImportOutput&, GraphicFilterImportFlags, BitmapScopedWriteAccess*,
BitmapScopedWriteAccess*)
/src/libreoffice/vcl/source/filter/png/PngImageReader.cxx:555:35
#8 0x567197efbb9e in read
/src/libreoffice/vcl/source/filter/png/PngImageReader.cxx:809:65
#9 0x567197efbb9e in vcl::PngImageReader::read()
/src/libreoffice/vcl/source/filter/png/PngImageReader.cxx:814:9
#10 0x567197b89b34 in LLVMFuzzerTestOneInput
/src/libreoffice/vcl/workben/pngfuzzer.cxx:52:19
#11 0x567197a7a1a0 in fuzzer::Fuzzer::ExecuteCallback(unsigned char
const*, unsigned long)
/src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:614:13
#12 0x567197a65535 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*,
unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:327:6
#13 0x567197a6afcf in fuzzer::FuzzerDriver(int*, char***, int
(*)(unsigned char const*, unsigned long))
/src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:862:9
#14 0x567197a95532 in main
/src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#15 0x7dffda1c4082 in __libc_start_main
/build/glibc-LcI20x/glibc-2.31/csu/libc-start.c:308:16
Change-Id: Id112c8be512b836e86a054075e7d5de83f5507c2
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/188721
Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>
Tested-by: Jenkins
diff --git a/vcl/source/filter/png/PngImageReader.cxx
b/vcl/source/filter/png/PngImageReader.cxx
index 977787e90e95..515b150ba054 100644
--- a/vcl/source/filter/png/PngImageReader.cxx
+++ b/vcl/source/filter/png/PngImageReader.cxx
@@ -10,6 +10,7 @@
#include <vcl/filter/PngImageReader.hxx>
#include <png.h>
+#include <iostream>
#include <rtl/crc.h>
#include <tools/stream.hxx>
#include <vcl/bitmap.hxx>
@@ -53,6 +54,11 @@ void lclReadStream(png_structp pPng, png_bytep pOutBytes,
png_size_t nBytesToRea
}
}
+void lclError(png_structp /*pPng*/, png_const_charp error_msg)
+{
+ throw std::runtime_error(error_msg);
+}
+
bool isPng(SvStream& rStream)
{
// Check signature bytes
@@ -324,10 +330,6 @@ bool fcTLbeforeIDAT(SvStream& rStream)
return false;
}
-#if defined __GNUC__ && __GNUC__ <= 14 && !defined __clang__
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wclobbered"
-#endif
bool reader(SvStream& rStream, ImportOutput& rImportOutput,
GraphicFilterImportFlags nImportFlags =
GraphicFilterImportFlags::NONE,
BitmapScopedWriteAccess* pAccess = nullptr,
@@ -369,219 +371,181 @@ bool reader(SvStream& rStream, ImportOutput&
rImportOutput,
const bool bUseExistingBitmap
= static_cast<bool>(nImportFlags &
GraphicFilterImportFlags::UseExistingBitmap);
- if (setjmp(png_jmpbuf(pPng)))
- {
- pWriteAccessInstance.reset();
- pWriteAccessAlphaInstance.reset();
- return false;
- }
+ int nNumberOfPasses(0);
+ int colorType = -1;
+ png_uint_32 width = 0;
+ png_uint_32 height = 0;
- png_set_option(pPng, PNG_MAXIMUM_INFLATE_WINDOW, PNG_OPTION_ON);
+ try
+ {
+ png_set_option(pPng, PNG_MAXIMUM_INFLATE_WINDOW, PNG_OPTION_ON);
- png_set_read_fn(pPng, &rStream, lclReadStream);
+ png_set_read_fn(pPng, &rStream, lclReadStream);
- if (!bFuzzing)
- png_set_crc_action(pPng, PNG_CRC_ERROR_QUIT, PNG_CRC_WARN_DISCARD);
- else
- png_set_crc_action(pPng, PNG_CRC_QUIET_USE, PNG_CRC_QUIET_USE);
+ png_set_error_fn(pPng, nullptr, lclError, nullptr);
- png_set_sig_bytes(pPng, PNG_SIGNATURE_SIZE);
+ if (!bFuzzing)
+ png_set_crc_action(pPng, PNG_CRC_ERROR_QUIT, PNG_CRC_WARN_DISCARD);
+ else
+ png_set_crc_action(pPng, PNG_CRC_QUIET_USE, PNG_CRC_QUIET_USE);
- png_read_info(pPng, pInfo);
+ png_set_sig_bytes(pPng, PNG_SIGNATURE_SIZE);
- png_uint_32 width = 0;
- png_uint_32 height = 0;
- int bitDepth = 0;
- int colorType = -1;
- int interlace = -1;
+ png_read_info(pPng, pInfo);
- png_uint_32 returnValue = png_get_IHDR(pPng, pInfo, &width, &height,
&bitDepth, &colorType,
- &interlace, nullptr, nullptr);
+ int bitDepth = 0;
+ int interlace = -1;
- if (returnValue != 1)
- return false;
+ png_uint_32 returnValue = png_get_IHDR(pPng, pInfo, &width, &height,
&bitDepth, &colorType,
+ &interlace, nullptr, nullptr);
- if (colorType == PNG_COLOR_TYPE_PALETTE)
- png_set_palette_to_rgb(pPng);
+ if (returnValue != 1)
+ return false;
- if (colorType == PNG_COLOR_TYPE_GRAY)
- png_set_expand_gray_1_2_4_to_8(pPng);
+ if (colorType == PNG_COLOR_TYPE_PALETTE)
+ png_set_palette_to_rgb(pPng);
- if (png_get_valid(pPng, pInfo, PNG_INFO_tRNS))
- png_set_tRNS_to_alpha(pPng);
+ if (colorType == PNG_COLOR_TYPE_GRAY)
+ png_set_expand_gray_1_2_4_to_8(pPng);
- if (bitDepth == 16)
- png_set_scale_16(pPng);
+ if (png_get_valid(pPng, pInfo, PNG_INFO_tRNS))
+ png_set_tRNS_to_alpha(pPng);
- if (bitDepth < 8)
- png_set_packing(pPng);
+ if (bitDepth == 16)
+ png_set_scale_16(pPng);
- // Convert gray+alpha to RGBA, keep gray as gray.
- if (colorType == PNG_COLOR_TYPE_GRAY_ALPHA
- || (colorType == PNG_COLOR_TYPE_GRAY && png_get_valid(pPng, pInfo,
PNG_INFO_tRNS)))
- {
- png_set_gray_to_rgb(pPng);
- }
+ if (bitDepth < 8)
+ png_set_packing(pPng);
- // Sets the filler byte - if RGB it converts to RGBA
- // png_set_filler(pPng, 0xFF, PNG_FILLER_AFTER);
+ // Convert gray+alpha to RGBA, keep gray as gray.
+ if (colorType == PNG_COLOR_TYPE_GRAY_ALPHA
+ || (colorType == PNG_COLOR_TYPE_GRAY && png_get_valid(pPng, pInfo,
PNG_INFO_tRNS)))
+ {
+ png_set_gray_to_rgb(pPng);
+ }
- int nNumberOfPasses = png_set_interlace_handling(pPng);
+ // Sets the filler byte - if RGB it converts to RGBA
+ // png_set_filler(pPng, 0xFF, PNG_FILLER_AFTER);
- png_read_update_info(pPng, pInfo);
- returnValue = png_get_IHDR(pPng, pInfo, &width, &height, &bitDepth,
&colorType, nullptr,
- nullptr, nullptr);
+ nNumberOfPasses = png_set_interlace_handling(pPng);
- if (returnValue != 1)
- return false;
+ png_read_update_info(pPng, pInfo);
+ returnValue = png_get_IHDR(pPng, pInfo, &width, &height, &bitDepth,
&colorType, nullptr,
+ nullptr, nullptr);
- if (bitDepth != 8
- || (colorType != PNG_COLOR_TYPE_RGB && colorType !=
PNG_COLOR_TYPE_RGB_ALPHA
- && colorType != PNG_COLOR_TYPE_GRAY))
- {
- return false;
- }
+ if (returnValue != 1)
+ return false;
- png_uint_32 res_x = 0;
- png_uint_32 res_y = 0;
- int unit_type = 0;
- if (png_get_pHYs(pPng, pInfo, &res_x, &res_y, &unit_type) != 0
- && unit_type == PNG_RESOLUTION_METER && res_x && res_y)
- {
- // convert into MapUnit::Map100thMM
- prefSize = Size(static_cast<sal_Int32>((100000.0 * width) / res_x),
- static_cast<sal_Int32>((100000.0 * height) / res_y));
- }
+ if (bitDepth != 8
+ || (colorType != PNG_COLOR_TYPE_RGB && colorType !=
PNG_COLOR_TYPE_RGB_ALPHA
+ && colorType != PNG_COLOR_TYPE_GRAY))
+ {
+ return false;
+ }
- if (!bUseExistingBitmap)
- {
- switch (colorType)
+ png_uint_32 res_x = 0;
+ png_uint_32 res_y = 0;
+ int unit_type = 0;
+ if (png_get_pHYs(pPng, pInfo, &res_x, &res_y, &unit_type) != 0
+ && unit_type == PNG_RESOLUTION_METER && res_x && res_y)
{
- case PNG_COLOR_TYPE_RGB:
- aBitmap = Bitmap(Size(width, height),
vcl::PixelFormat::N24_BPP);
- break;
- case PNG_COLOR_TYPE_RGBA:
- aBitmap = Bitmap(Size(width, height),
vcl::PixelFormat::N24_BPP);
- aBitmapAlpha = AlphaMask(Size(width, height), nullptr);
- aBitmapAlpha.Erase(0); // opaque
- break;
- case PNG_COLOR_TYPE_GRAY:
- aBitmap = Bitmap(Size(width, height), vcl::PixelFormat::N8_BPP,
- &Bitmap::GetGreyPalette(256));
- break;
- default:
- abort();
+ // convert into MapUnit::Map100thMM
+ prefSize = Size(static_cast<sal_Int32>((100000.0 * width) / res_x),
+ static_cast<sal_Int32>((100000.0 * height) /
res_y));
}
- if (bOnlyCreateBitmap)
+ if (!bUseExistingBitmap)
{
- if (!aBitmapAlpha.IsEmpty())
- aBitmapEx = BitmapEx(aBitmap, aBitmapAlpha);
- else
- aBitmapEx = BitmapEx(aBitmap);
- if (!prefSize.IsEmpty())
+ switch (colorType)
{
- aBitmapEx.SetPrefMapMode(MapMode(MapUnit::Map100thMM));
- aBitmapEx.SetPrefSize(prefSize);
+ case PNG_COLOR_TYPE_RGB:
+ aBitmap = Bitmap(Size(width, height),
vcl::PixelFormat::N24_BPP);
+ break;
+ case PNG_COLOR_TYPE_RGBA:
+ aBitmap = Bitmap(Size(width, height),
vcl::PixelFormat::N24_BPP);
+ aBitmapAlpha = AlphaMask(Size(width, height), nullptr);
+ aBitmapAlpha.Erase(0); // opaque
+ break;
+ case PNG_COLOR_TYPE_GRAY:
+ aBitmap = Bitmap(Size(width, height),
vcl::PixelFormat::N8_BPP,
+ &Bitmap::GetGreyPalette(256));
+ break;
+ default:
+ abort();
}
- rImportOutput.moBitmap = aBitmapEx;
- return true;
- }
- pWriteAccessInstance = aBitmap;
- if (!pWriteAccessInstance)
- return false;
- if (!aBitmapAlpha.IsEmpty())
- {
- pWriteAccessAlphaInstance = aBitmapAlpha;
- if (!pWriteAccessAlphaInstance)
+ if (bOnlyCreateBitmap)
+ {
+ if (!aBitmapAlpha.IsEmpty())
+ aBitmapEx = BitmapEx(aBitmap, aBitmapAlpha);
+ else
+ aBitmapEx = BitmapEx(aBitmap);
+ if (!prefSize.IsEmpty())
+ {
+ aBitmapEx.SetPrefMapMode(MapMode(MapUnit::Map100thMM));
+ aBitmapEx.SetPrefSize(prefSize);
+ }
+ rImportOutput.moBitmap = aBitmapEx;
+ return true;
+ }
+
+ pWriteAccessInstance = aBitmap;
+ if (!pWriteAccessInstance)
return false;
+ if (!aBitmapAlpha.IsEmpty())
+ {
+ pWriteAccessAlphaInstance = aBitmapAlpha;
+ if (!pWriteAccessAlphaInstance)
+ return false;
+ }
}
}
- BitmapScopedWriteAccess& pWriteAccess = pAccess ? *pAccess :
pWriteAccessInstance;
- BitmapScopedWriteAccess& pWriteAccessAlpha
- = pAlphaAccess ? *pAlphaAccess : pWriteAccessAlphaInstance;
-
- if (setjmp(png_jmpbuf(pPng)))
+ catch (const std::runtime_error& ex)
{
+ std::cerr << "libpng error: " << ex.what() << std::endl;
+
pWriteAccessInstance.reset();
pWriteAccessAlphaInstance.reset();
-
- if (!bUseExistingBitmap)
- {
- // Set the bitmap if it contains something, even on failure. This
allows
- // reading images that are only partially broken.
- pWriteAccessInstance.reset();
- pWriteAccessAlphaInstance.reset();
- if (!aBitmap.IsEmpty() && !aBitmapAlpha.IsEmpty())
- aBitmapEx = BitmapEx(aBitmap, aBitmapAlpha);
- else if (!aBitmap.IsEmpty())
- aBitmapEx = BitmapEx(aBitmap);
- if (!aBitmapEx.IsEmpty() && !prefSize.IsEmpty())
- {
- aBitmapEx.SetPrefMapMode(MapMode(MapUnit::Map100thMM));
- aBitmapEx.SetPrefSize(prefSize);
- }
- rImportOutput.moBitmap = aBitmapEx;
- }
return false;
}
- if (colorType == PNG_COLOR_TYPE_RGB)
- {
- ScanlineFormat eFormat = pWriteAccess->GetScanlineFormat();
- if (eFormat == ScanlineFormat::N24BitTcBgr)
- png_set_bgr(pPng);
+ BitmapScopedWriteAccess& pWriteAccess = pAccess ? *pAccess :
pWriteAccessInstance;
+ BitmapScopedWriteAccess& pWriteAccessAlpha
+ = pAlphaAccess ? *pAlphaAccess : pWriteAccessAlphaInstance;
- for (int pass = 0; pass < nNumberOfPasses; pass++)
- {
- for (png_uint_32 y = 0; y < height; y++)
- {
- Scanline pScanline = pWriteAccess->GetScanline(y);
- png_read_row(pPng, pScanline, nullptr);
- }
- }
- }
- else if (colorType == PNG_COLOR_TYPE_RGB_ALPHA)
+ try
{
- size_t aRowSizeBytes = png_get_rowbytes(pPng, pInfo);
- ScanlineFormat eFormat = pWriteAccess->GetScanlineFormat();
- if (eFormat == ScanlineFormat::N24BitTcBgr)
- png_set_bgr(pPng);
-
- if (nNumberOfPasses == 1)
+ if (colorType == PNG_COLOR_TYPE_RGB)
{
- // optimise the common case, where we can use a buffer of only a
single row
- std::vector<png_byte> aRow(aRowSizeBytes, 0);
- for (png_uint_32 y = 0; y < height; y++)
+ ScanlineFormat eFormat = pWriteAccess->GetScanlineFormat();
+ if (eFormat == ScanlineFormat::N24BitTcBgr)
+ png_set_bgr(pPng);
+
+ for (int pass = 0; pass < nNumberOfPasses; pass++)
{
- Scanline pScanline = pWriteAccess->GetScanline(y);
- Scanline pScanAlpha = pWriteAccessAlpha->GetScanline(y);
- png_bytep pRow = aRow.data();
- png_read_row(pPng, pRow, nullptr);
- size_t iAlpha = 0;
- size_t iColor = 0;
- for (size_t i = 0; i < aRowSizeBytes; i += 4)
+ for (png_uint_32 y = 0; y < height; y++)
{
- pScanline[iColor++] = pRow[i + 0];
- pScanline[iColor++] = pRow[i + 1];
- pScanline[iColor++] = pRow[i + 2];
- pScanAlpha[iAlpha++] = pRow[i + 3];
+ Scanline pScanline = pWriteAccess->GetScanline(y);
+ png_read_row(pPng, pScanline, nullptr);
}
}
}
- else
+ else if (colorType == PNG_COLOR_TYPE_RGB_ALPHA)
{
- std::vector<std::vector<png_byte>> aRows(height);
- for (auto& rRow : aRows)
- rRow.resize(aRowSizeBytes, 0);
- for (int pass = 0; pass < nNumberOfPasses; pass++)
+ size_t aRowSizeBytes = png_get_rowbytes(pPng, pInfo);
+ ScanlineFormat eFormat = pWriteAccess->GetScanlineFormat();
+ if (eFormat == ScanlineFormat::N24BitTcBgr)
+ png_set_bgr(pPng);
+
+ if (nNumberOfPasses == 1)
{
+ // optimise the common case, where we can use a buffer of only
a single row
+ std::vector<png_byte> aRow(aRowSizeBytes, 0);
for (png_uint_32 y = 0; y < height; y++)
{
Scanline pScanline = pWriteAccess->GetScanline(y);
Scanline pScanAlpha = pWriteAccessAlpha->GetScanline(y);
- png_bytep pRow = aRows[y].data();
+ png_bytep pRow = aRow.data();
png_read_row(pPng, pRow, nullptr);
size_t iAlpha = 0;
size_t iColor = 0;
@@ -594,23 +558,79 @@ bool reader(SvStream& rStream, ImportOutput&
rImportOutput,
}
}
}
+ else
+ {
+ std::vector<std::vector<png_byte>> aRows(height);
+ for (auto& rRow : aRows)
+ rRow.resize(aRowSizeBytes, 0);
+ for (int pass = 0; pass < nNumberOfPasses; pass++)
+ {
+ for (png_uint_32 y = 0; y < height; y++)
+ {
+ Scanline pScanline = pWriteAccess->GetScanline(y);
+ Scanline pScanAlpha =
pWriteAccessAlpha->GetScanline(y);
+ png_bytep pRow = aRows[y].data();
+ png_read_row(pPng, pRow, nullptr);
+ size_t iAlpha = 0;
+ size_t iColor = 0;
+ for (size_t i = 0; i < aRowSizeBytes; i += 4)
+ {
+ pScanline[iColor++] = pRow[i + 0];
+ pScanline[iColor++] = pRow[i + 1];
+ pScanline[iColor++] = pRow[i + 2];
+ pScanAlpha[iAlpha++] = pRow[i + 3];
+ }
+ }
+ }
+ }
+ }
+ else if (colorType == PNG_COLOR_TYPE_GRAY)
+ {
+ for (int pass = 0; pass < nNumberOfPasses; pass++)
+ {
+ for (png_uint_32 y = 0; y < height; y++)
+ {
+ Scanline pScanline = pWriteAccess->GetScanline(y);
+ png_read_row(pPng, pScanline, nullptr);
+ }
+ }
}
}
- else if (colorType == PNG_COLOR_TYPE_GRAY)
+ catch (const std::runtime_error& ex)
{
- for (int pass = 0; pass < nNumberOfPasses; pass++)
+ std::cerr << "libpng error: " << ex.what() << std::endl;
+
+ pWriteAccessInstance.reset();
+ pWriteAccessAlphaInstance.reset();
+
+ if (!bUseExistingBitmap)
{
- for (png_uint_32 y = 0; y < height; y++)
+ // Set the bitmap if it contains something, even on failure. This
allows
+ // reading images that are only partially broken.
+ pWriteAccessInstance.reset();
+ pWriteAccessAlphaInstance.reset();
+ if (!aBitmap.IsEmpty() && !aBitmapAlpha.IsEmpty())
+ aBitmapEx = BitmapEx(aBitmap, aBitmapAlpha);
+ else if (!aBitmap.IsEmpty())
+ aBitmapEx = BitmapEx(aBitmap);
+ if (!aBitmapEx.IsEmpty() && !prefSize.IsEmpty())
{
- Scanline pScanline = pWriteAccess->GetScanline(y);
- png_read_row(pPng, pScanline, nullptr);
+ aBitmapEx.SetPrefMapMode(MapMode(MapUnit::Map100thMM));
+ aBitmapEx.SetPrefSize(prefSize);
}
+ rImportOutput.moBitmap = aBitmapEx;
}
+ return false;
}
-
// If an error is thrown when calling png_read_end
- if (setjmp(png_jmpbuf(pPng)))
+ try
{
+ png_read_end(pPng, pInfo);
+ }
+ catch (const std::runtime_error& ex)
+ {
+ std::cerr << "libpng error: " << ex.what() << std::endl;
+
if (!bUseExistingBitmap)
{
pWriteAccessInstance.reset();
@@ -630,8 +650,6 @@ bool reader(SvStream& rStream, ImportOutput& rImportOutput,
return true;
}
- png_read_end(pPng, pInfo);
-
if (!bUseExistingBitmap)
{
pWriteAccess.reset();
@@ -784,9 +802,6 @@ BinaryDataContainer getMsGifChunk(SvStream& rStream)
return {};
}
}
-#if defined __GNUC__ && __GNUC__ <= 14 && !defined __clang__
-#pragma GCC diagnostic pop
-#endif
} // anonymous namespace
