Bug #638
Valgrind reports errors when writing to PNG image
100%
Description
Running the exiv2 command given in #630 on a PNG image without any metadata causes Valgrind to complain:
ahuggel@kelana> valgrind ./exiv2 -M"set Exif.Photo.UserComment charset=Ascii New Exif comment" wo_metadata.png ==12883== Memcheck, a memory error detector. ==12883== Copyright (C) 2002-2008, and GNU GPL'd, by Julian Seward et al. ==12883== Using LibVEX rev 1878, a library for dynamic binary translation. ==12883== Copyright (C) 2004-2008, and GNU GPL'd, by OpenWorks LLP. ==12883== Using valgrind-3.4.0-Debian, a dynamic binary instrumentation framework. ==12883== Copyright (C) 2000-2008, and GNU GPL'd, by Julian Seward et al. ==12883== For more details, rerun with: -v ==12883== ==12883== Invalid read of size 1 ==12883== at 0x457317: Exiv2::Internal::PngChunk::copyString(char*, char const*, unsigned long) (pngchunk.cpp:773) ==12883== by 0x457631: Exiv2::Internal::PngChunk::writeRawProfile(Exiv2::DataBuf const&, Exiv2::DataBuf const&) (pngchunk.cpp:718) ==12883== by 0x458AE4: Exiv2::Internal::PngChunk::makeMetadataChunk(Exiv2::DataBuf const&, Exiv2::Internal::PngChunk::MetadataType, bool) (pngchunk.cpp:373) ==12883== by 0x455517: Exiv2::PngImage::doWriteMetadata(Exiv2::BasicIo&) (pngimage.cpp:283) ==12883== by 0x4563D4: Exiv2::PngImage::writeMetadata() (pngimage.cpp:188) ==12883== by 0x4170B0: Action::Modify::run(std::string const&) (actions.cpp:1159) ==12883== by 0x40B911: main (exiv2.cpp:165) ==12883== Address 0x5f99d6c is 0 bytes after a block of size 4 alloc'd ==12883== at 0x4C21B9C: operator new[](unsigned long) (vg_replace_malloc.c:274) ==12883== by 0x4233AE: Exiv2::DataBuf::DataBuf(long) (types.hpp:206) ==12883== by 0x458AB4: Exiv2::Internal::PngChunk::makeMetadataChunk(Exiv2::DataBuf const&, Exiv2::Internal::PngChunk::MetadataType, bool) (pngchunk.cpp:371) ==12883== by 0x455517: Exiv2::PngImage::doWriteMetadata(Exiv2::BasicIo&) (pngimage.cpp:283) ==12883== by 0x4563D4: Exiv2::PngImage::writeMetadata() (pngimage.cpp:188) ==12883== by 0x4170B0: Action::Modify::run(std::string const&) (actions.cpp:1159) ==12883== by 0x40B911: main (exiv2.cpp:165) ==12883== ==12883== Conditional jump or move depends on uninitialised value(s) ==12883== at 0x4C230A8: strlen (mc_replace_strmem.c:242) ==12883== by 0x457659: Exiv2::Internal::PngChunk::writeRawProfile(Exiv2::DataBuf const&, Exiv2::DataBuf const&) (pngchunk.cpp:723) ==12883== by 0x458AE4: Exiv2::Internal::PngChunk::makeMetadataChunk(Exiv2::DataBuf const&, Exiv2::Internal::PngChunk::MetadataType, bool) (pngchunk.cpp:373) ==12883== by 0x455517: Exiv2::PngImage::doWriteMetadata(Exiv2::BasicIo&) (pngimage.cpp:283) ==12883== by 0x4563D4: Exiv2::PngImage::writeMetadata() (pngimage.cpp:188) ==12883== by 0x4170B0: Action::Modify::run(std::string const&) (actions.cpp:1159) ==12883== by 0x40B911: main (exiv2.cpp:165) ==12883== ==12883== Use of uninitialised value of size 8 ==12883== at 0x4E2ECDB: (within /usr/lib/libz.so.1.2.3.3) ==12883== by 0x4E307B0: (within /usr/lib/libz.so.1.2.3.3) ==12883== by 0x4E2D5FB: (within /usr/lib/libz.so.1.2.3.3) ==12883== by 0x4E2C28C: deflate (in /usr/lib/libz.so.1.2.3.3) ==12883== by 0x4E28951: compress2 (in /usr/lib/libz.so.1.2.3.3) ==12883== by 0x457E3D: Exiv2::Internal::PngChunk::zlibCompress(unsigned char const*, unsigned int, Exiv2::DataBuf&) (pngchunk.cpp:457) ==12883== by 0x458456: Exiv2::Internal::PngChunk::makeAsciiTxtChunk(Exiv2::DataBuf const&, Exiv2::DataBuf const&, bool) (pngchunk.cpp:507) ==12883== by 0x458BEF: Exiv2::Internal::PngChunk::makeMetadataChunk(Exiv2::DataBuf const&, Exiv2::Internal::PngChunk::MetadataType, bool) (pngchunk.cpp:377) ==12883== by 0x455517: Exiv2::PngImage::doWriteMetadata(Exiv2::BasicIo&) (pngimage.cpp:283) ==12883== by 0x4563D4: Exiv2::PngImage::writeMetadata() (pngimage.cpp:188) ==12883== by 0x4170B0: Action::Modify::run(std::string const&) (actions.cpp:1159) ==12883== by 0x40B911: main (exiv2.cpp:165) ==12883== ==12883== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 25 from 3) ==12883== malloc/free: in use at exit: 0 bytes in 0 blocks. ==12883== malloc/free: 290 allocs, 290 frees, 382,619 bytes allocated. ==12883== For counts of detected errors, rerun with: -v ==12883== Use --track-origins=yes to see where uninitialised values come from ==12883== All heap blocks were freed -- no leaks are possible.
Related issues
Associated revisions
#638: Fixed compiler warnings, fixed more CRC issues, shortened some code.
#638: Embed IPTC data in Photoshop IRB, some more code re-work.
#638: Aligned IPTC decoding with the new encoding, but kept it backward compatible for broken images.
History
Updated by Andreas Huggel over 12 years ago
r1817 fixes the Valgrind errors. But there is more work to do:
- requires testing on Windows (type conversions)
- fix compiler warnings
- fix the todo left in the code
Noticed another issue during testing. After running this command
exiv2 -M"set Iptc.Application2.Keywords digiKam" wo_metadata.png
exiftool reports a
Bad Photoshop IRB resource "\x1c\x02\x19\x00"
and doesn't display any IPTC values. (Exiv2 displays the keyword.) This is not a new issue, I can reproduce it with 0.18.1.
Gilles, could you please also test with this revision? I've only done some basic read and write tests (Exif, IPTC and XMP) using the exiv2 command line tool.
Updated by Gilles Caulier over 12 years ago
Andreas Huggel wrote:
r1817 fixes the Valgrind errors. But there is more work to do:
- requires testing on Windows (type conversions)
- fix compiler warnings
- fix the todo left in the code
Noticed another issue during testing. After running this command
[...]
exiftool reports a
[...]
and doesn't display any IPTC values. (Exiv2 displays the keyword.) This is not a new issue, I can reproduce it with 0.18.1.
Gilles, could you please also test with this revision? I've only done some basic read and write tests (Exif, IPTC and XMP) using the exiv2 command line tool.
No, it do not work :
digikam(27167)/digikam (core) Digikam::DImg::load: "/mnt/data/photo/TEST_IMAGES/METADATA/Vista/Garden.png" : PNG file identified
libpng warning: iTXt: CRC error
Gilles
Updated by Gilles Caulier over 12 years ago
Gilles Caulier wrote:
Andreas Huggel wrote:
r1817 fixes the Valgrind errors. But there is more work to do:
- requires testing on Windows (type conversions)
- fix compiler warnings
- fix the todo left in the code
Noticed another issue during testing. After running this command
[...]
exiftool reports a
[...]
and doesn't display any IPTC values. (Exiv2 displays the keyword.) This is not a new issue, I can reproduce it with 0.18.1.
Gilles, could you please also test with this revision? I've only done some basic read and write tests (Exif, IPTC and XMP) using the exiv2 command line tool.
No, it do not work :
digikam(27167)/digikam (core) Digikam::DImg::load: "/mnt/data/photo/TEST_IMAGES/METADATA/Vista/Garden.png" : PNG file identified
libpng warning: iTXt: CRC errorGilles
Impossible to attach PNG file with redmine. I report a problem.
I send you file by mail
Gilles
Updated by Andreas Huggel over 12 years ago
- % Done changed from 60 to 70
r1818 fixes the compiler warnings, the todo that was left and more CRC issues. The IPTC Photoshop IRB issue is still outstanding and more code beautification is possible.
Gilles, this revision should also fix the iTXt CRC warning (the initial fix only addressed the CRC issue for one of the ASCII text chunks). Since I only modified the PNG write-code, I can't really test with the image you sent, it's better if you repeat the steps with digiKam.
Updated by Gilles Caulier over 12 years ago
Andreas Huggel wrote:
r1818 fixes the compiler warnings, the todo that was left and more CRC issues. The IPTC Photoshop IRB issue is still outstanding and more code beautification is possible.
Gilles, this revision should also fix the iTXt CRC warning (the initial fix only addressed the CRC issue for one of the ASCII text chunks). Since I only modified the PNG write-code, I can't really test with the image you sent, it's better if you repeat the steps with digiKam.
Andreas,
r1818 fix the problem definitively. thanks.
Gilles Caulier
Updated by Andreas Huggel over 12 years ago
- Status changed from New to Resolved
- % Done changed from 70 to 100
Fixed remaining IPTC Photoshop IRB issue. For backward compatibility, PNGs with the old broken IPTC chunk are still decoded. I'm closing this bug now.
Testing and feedback appreciated, I ended up changing quite a bit of the PNG write code by now.
#638: Replaced custom copyString function with strcpy, refactored some related code. Only tested on Linux, this may not compile on Windows.