memcpy-param-overlap bug in src/tiffvisitor_int.cpp

[yuawn]

Hi @clanmills @kevinbackhouse,

There is a memcpy-param-overlap bug in src/tiffvisitor_int.cpp:748:17, and the bug is reproducible on the latest release v0.27.5 and main 65fad9d.

Thank you

Describe the bug
memcpy-param-overlap in src/tiffvisitor_int.cpp:748:17 (Exiv2::Internal::TiffEncoder::updateDirEntry(unsigned char*, Exiv2::ByteOrder, Exiv2::Internal::TiffComponent*))

PoC
poc.gz

Environment:
Ubuntu 20.04.3 LTS (x86_64)
clang version 13.0.1 (https://github.com/llvm/llvm-project.git 19b8368225dc9ec5a0da547eae48c10dae13522d)

To Reproduce

cd exiv2
mkdir build-fuzz; cd build-fuzz

cmake -DEXIV2_ENABLE_PNG=ON -DEXIV2_ENABLE_WEBREADY=ON -DEXIV2_ENABLE_CURL=ON -DEXIV2_ENABLE_BMFF=ON -DEXIV2_TEAM_WARNINGS_AS_ERRORS=ON -DCMAKE_CXX_COMPILER=$(which clang++) -DEXIV2_BUILD_FUZZ_TESTS=ON -DEXIV2_TEAM_USE_SANITIZERS=ON ..
make -j $(nproc)

./bin/fuzz-read-print-write ./poc

AddressSanitizer Report:

=================================================================
==341184==ERROR: AddressSanitizer: memcpy-param-overlap: memory ranges [0x611000000762,0x611000000766) and [0x611000000765, 0x611000000769) overlap
    #0 0x5646bf in __asan_memcpy llvm-project/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:22:3
    #1 0x7ffff7624054 in memcpy /usr/include/x86_64-linux-gnu/bits/string_fortified.h:34:10
    #2 0x7ffff7624054 in Exiv2::Internal::TiffEncoder::updateDirEntry(unsigned char*, Exiv2::ByteOrder, Exiv2::Internal::TiffComponent*) src/tiffvisitor_int.cpp:748:17
    #3 0x7ffff76236ab in Exiv2::Internal::TiffEncoder::visitDirectoryNext(Exiv2::Internal::TiffDirectory*) src/tiffvisitor_int.cpp:728:18
    #4 0x7ffff75c3e38 in Exiv2::Internal::TiffDirectory::doAccept(Exiv2::Internal::TiffVisitor&) src/tiffcomposite_int.cpp:887:58
    #5 0x7ffff7601de9 in Exiv2::Internal::TiffParserWorker::encode(Exiv2::BasicIo&, unsigned char const*, unsigned int, Exiv2::ExifData const&, Exiv2::IptcData const&, Exiv2::XmpData const&, unsigned int, void (Exiv2::Internal::TiffEncoder::* (*)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, unsigned int, Exiv2::Internal::IfdId))(Exiv2::Internal::TiffEntryBase*, Exiv2::Exifdatum const*), Exiv2::Internal::TiffHeaderBase*, Exiv2::Internal::OffsetWriter*) src/tiffimage_int.cpp:1947:25
    #6 0x7ffff71c747b in Exiv2::ExifParser::encode(std::vector<unsigned char, std::allocator<unsigned char> >&, unsigned char const*, unsigned int, Exiv2::ByteOrder, Exiv2::ExifData const&) src/exif.cpp:751:26
    #7 0x7ffff7267447 in Exiv2::JpegBase::doWriteMetadata(Exiv2::BasicIo&) src/jpgimage.cpp:1052:38
    #8 0x7ffff7263b45 in Exiv2::JpegBase::writeMetadata() src/jpgimage.cpp:860:9
    #9 0x5a20a4 in LLVMFuzzerTestOneInput fuzz/fuzz-read-print-write.cpp:44:12
    #10 0x4660cc in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:17
    #11 0x451a33 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:323:21
    #12 0x45815b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:856:19
    #13 0x422296 in main llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:30
    #14 0x7ffff66a10b2 in __libc_start_main /build/glibc-YbNSs7/glibc-2.31/csu/../csu/libc-start.c:308:16
    #15 0x42233d in _start (build/bin/fuzz-read-print-write+0x42233d)

0x611000000762 is located 226 bytes inside of 248-byte region [0x611000000680,0x611000000778)
allocated by thread T0 here:
    #0 0x565bd7 in malloc llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:129:3
    #1 0x43ed1e in operator new(unsigned long) (build/bin/fuzz-read-print-write+0x43ed1e)
    #2 0x7ffff7265db3 in Exiv2::JpegBase::doWriteMetadata(Exiv2::BasicIo&) src/jpgimage.cpp:939:29
    #3 0x7ffff7263b45 in Exiv2::JpegBase::writeMetadata() src/jpgimage.cpp:860:9
    #4 0x5a20a4 in LLVMFuzzerTestOneInput fuzz/fuzz-read-print-write.cpp:44:12
    #5 0x4660cc in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:17
    #6 0x451a33 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:323:21
    #7 0x45815b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:856:19
    #8 0x422296 in main llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:30
    #9 0x7ffff66a10b2 in __libc_start_main /build/glibc-YbNSs7/glibc-2.31/csu/../csu/libc-start.c:308:16

0x611000000765 is located 229 bytes inside of 248-byte region [0x611000000680,0x611000000778)
allocated by thread T0 here:
    #0 0x565bd7 in malloc llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:129:3
    #1 0x43ed1e in operator new(unsigned long) (build/bin/fuzz-read-print-write+0x43ed1e)
    #2 0x7ffff7265db3 in Exiv2::JpegBase::doWriteMetadata(Exiv2::BasicIo&) src/jpgimage.cpp:939:29
    #3 0x7ffff7263b45 in Exiv2::JpegBase::writeMetadata() src/jpgimage.cpp:860:9
    #4 0x5a20a4 in LLVMFuzzerTestOneInput fuzz/fuzz-read-print-write.cpp:44:12
    #5 0x4660cc in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:17
    #6 0x451a33 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:323:21
    #7 0x45815b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:856:19
    #8 0x422296 in main llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:30
    #9 0x7ffff66a10b2 in __libc_start_main /build/glibc-YbNSs7/glibc-2.31/csu/../csu/libc-start.c:308:16

SUMMARY: AddressSanitizer: memcpy-param-overlap llvm-project/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:22:3 in __asan_memcpy
==341184==ABORTING

[kevinbackhouse]

Thanks @yuawn!

[kevinbackhouse]

Background info: we initially investigated this as a potential security vulnerability, but have now concluded that it's a regular bug with no security impact. So I asked @yuawn to create this public issue.

[kevinbackhouse]

The bug is here:

exiv2/src/tiffvisitor_int.cpp

Lines 747 to 750 in fd84471

	if (pTiffEntry->size() > 0) {
	memcpy(buf + 8, pTiffEntry->pData(), pTiffEntry->size());
	memset(const_cast<byte*>(pTiffEntry->pData()), 0x0, pTiffEntry->size());
	}

buf and pTiffEntry->pData() are both pointers into the same (larger) buffer. As the poc shows, it is possible for the two buffers to overlap. Therefore, we should use memmove rather than memcpy here.

I don't believe this bug is a security concern. The code is just moving data within a larger buffer, so the access isn't out-of-bounds. The data might get copied incorrectly, but it's already attacker-controlled data so that doesn't make any difference from a security perspective.