search

google / pik

A new lossy/lossless image format for photos and the internet
MIT License
834 stars 51 forks source link

heap-buffer-overflow in pik::DecodeImageData #16

Closed gy741 closed 7 years ago

gy741 commented 7 years ago

Hi.

I found a heap-buffer-overflow bug in pik.

Please confirm.

Thanks.

Summary: heap-buffer-overflow Browser/OS: Ubuntu 17.04 64bit Steps to reproduce: 1.Download the .POC files. 2.Compile the source code with ASan. 3.Execute the following command : ./dpik $PoC /dev/null PoC download : PoC

------------
Valgrind Log
------------
==29337== Use of uninitialised value of size 8
==29337==    at 0x13B7EC: Advance (bit_reader.h:54)
==29337==    by 0x13B7EC: pik::ReadHistogram(int, std::vector<int, std::allocator<int> >*, pik::BitReader*) (histogram_decode.cc:88)
==29337==    by 0x14C6D8: DecodeHistograms (opsin_codec.cc:672)
==29337==    by 0x14C6D8: pik::DecodeHistograms(unsigned char const*, unsigned long, unsigned long, unsigned char const*, unsigned long, pik::ANSSymbolReader*, std::vector<unsigned char, std::allocator<unsigned char> >*) (opsin_codec.cc:733)
==29337==    by 0x14CC34: pik::DecodeImage(unsigned char const*, unsigned long, int, pik::Image3<short>*) (opsin_codec.cc:882)
==29337==    by 0x11CCE9: pik::CompressedImage::Decode(int, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, pik::PikInfo*) (compressed_image.cc:394)
==29337==    by 0x1266D0: pik::PikToPixels(pik::DecompressParams const&, std::vector<unsigned char, std::allocator<unsigned char> > const&, pik::Image3<unsigned char>*, pik::PikInfo*) (pik.cc:544)
==29337==    by 0x10A89D: pik::(anonymous namespace)::Decompress(char const*, char const*) (dpik.cc:58)
==29337==    by 0x5DBC3F0: (below main) (libc-start.c:291)
==29337==
==29337== Conditional jump or move depends on uninitialised value(s)
==29337==    at 0x13B8FD: pik::ReadHistogram(int, std::vector<int, std::allocator<int> >*, pik::BitReader*) (histogram_decode.cc:112)
==29337==    by 0x14C6D8: DecodeHistograms (opsin_codec.cc:672)
==29337==    by 0x14C6D8: pik::DecodeHistograms(unsigned char const*, unsigned long, unsigned long, unsigned char const*, unsigned long, pik::ANSSymbolReader*, std::vector<unsigned char, std::allocator<unsigned char> >*) (opsin_codec.cc:733)
==29337==    by 0x14CC34: pik::DecodeImage(unsigned char const*, unsigned long, int, pik::Image3<short>*) (opsin_codec.cc:882)
==29337==    by 0x11CCE9: pik::CompressedImage::Decode(int, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, pik::PikInfo*) (compressed_image.cc:394)
==29337==    by 0x1266D0: pik::PikToPixels(pik::DecompressParams const&, std::vector<unsigned char, std::allocator<unsigned char> > const&, pik::Image3<unsigned char>*, pik::PikInfo*) (pik.cc:544)
==29337==    by 0x10A89D: pik::(anonymous namespace)::Decompress(char const*, char const*) (dpik.cc:58)
==29337==    by 0x5DBC3F0: (below main) (libc-start.c:291)
==29337==
==29337== Conditional jump or move depends on uninitialised value(s)
==29337==    at 0x14C765: DecodeHistograms (opsin_codec.cc:683)
==29337==    by 0x14C765: pik::DecodeHistograms(unsigned char const*, unsigned long, unsigned long, unsigned char const*, unsigned long, pik::ANSSymbolReader*, std::vector<unsigned char, std::allocator<unsigned char> >*) (opsin_codec.cc:733)
==29337==    by 0x14CC34: pik::DecodeImage(unsigned char const*, unsigned long, int, pik::Image3<short>*) (opsin_codec.cc:882)
==29337==    by 0x11CCE9: pik::CompressedImage::Decode(int, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, pik::PikInfo*) (compressed_image.cc:394)
==29337==    by 0x1266D0: pik::PikToPixels(pik::DecompressParams const&, std::vector<unsigned char, std::allocator<unsigned char> > const&, pik::Image3<unsigned char>*, pik::PikInfo*) (pik.cc:544)
==29337==    by 0x10A89D: pik::(anonymous namespace)::Decompress(char const*, char const*) (dpik.cc:58)
==29337==    by 0x5DBC3F0: (below main) (libc-start.c:291)
==29337==
==29337== Conditional jump or move depends on uninitialised value(s)
==29337==    at 0x14C78A: DecodeHistograms (opsin_codec.cc:683)
==29337==    by 0x14C78A: pik::DecodeHistograms(unsigned char const*, unsigned long, unsigned long, unsigned char const*, unsigned long, pik::ANSSymbolReader*, std::vector<unsigned char, std::allocator<unsigned char> >*) (opsin_codec.cc:733)
==29337==    by 0x14CC34: pik::DecodeImage(unsigned char const*, unsigned long, int, pik::Image3<short>*) (opsin_codec.cc:882)
==29337==    by 0x11CCE9: pik::CompressedImage::Decode(int, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, pik::PikInfo*) (compressed_image.cc:394)
==29337==    by 0x1266D0: pik::PikToPixels(pik::DecompressParams const&, std::vector<unsigned char, std::allocator<unsigned char> > const&, pik::Image3<unsigned char>*, pik::PikInfo*) (pik.cc:544)
==29337==    by 0x10A89D: pik::(anonymous namespace)::Decompress(char const*, char const*) (dpik.cc:58)
==29337==    by 0x5DBC3F0: (below main) (libc-start.c:291)
==29337==
==29337== Conditional jump or move depends on uninitialised value(s)
==29337==    at 0x1496D2: ReadSymbol (opsin_codec.cc:702)
==29337==    by 0x1496D2: pik::DecodeImageData(unsigned char const*, unsigned long, std::vector<unsigned char, std::allocator<unsigned char> > const&, int, pik::ANSSymbolReader*, pik::Image3<short>*) (opsin_codec.cc:751)
==29337==    by 0x14CC5D: pik::DecodeImage(unsigned char const*, unsigned long, int, pik::Image3<short>*) (opsin_codec.cc:884)
==29337==    by 0x11CCE9: pik::CompressedImage::Decode(int, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, pik::PikInfo*) (compressed_image.cc:394)
==29337==    by 0x1266D0: pik::PikToPixels(pik::DecompressParams const&, std::vector<unsigned char, std::allocator<unsigned char> > const&, pik::Image3<unsigned char>*, pik::PikInfo*) (pik.cc:544)
==29337==    by 0x10A89D: pik::(anonymous namespace)::Decompress(char const*, char const*) (dpik.cc:58)
==29337==    by 0x5DBC3F0: (below main) (libc-start.c:291)
==29337==
==29337== Use of uninitialised value of size 8
==29337==    at 0x1496A7: ReadSymbol (opsin_codec.cc:698)
==29337==    by 0x1496A7: pik::DecodeImageData(unsigned char const*, unsigned long, std::vector<unsigned char, std::allocator<unsigned char> > const&, int, pik::ANSSymbolReader*, pik::Image3<short>*) (opsin_codec.cc:751)
==29337==    by 0x14CC5D: pik::DecodeImage(unsigned char const*, unsigned long, int, pik::Image3<short>*) (opsin_codec.cc:884)
==29337==    by 0x11CCE9: pik::CompressedImage::Decode(int, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, pik::PikInfo*) (compressed_image.cc:394)
==29337==    by 0x1266D0: pik::PikToPixels(pik::DecompressParams const&, std::vector<unsigned char, std::allocator<unsigned char> > const&, pik::Image3<unsigned char>*, pik::PikInfo*) (pik.cc:544)
==29337==    by 0x10A89D: pik::(anonymous namespace)::Decompress(char const*, char const*) (dpik.cc:58)
==29337==    by 0x5DBC3F0: (below main) (libc-start.c:291)
==29337==
==29337== Conditional jump or move depends on uninitialised value(s)
==29337==    at 0x14CC68: pik::DecodeImage(unsigned char const*, unsigned long, int, pik::Image3<short>*) (opsin_codec.cc:886)
==29337==    by 0x11CCE9: pik::CompressedImage::Decode(int, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, pik::PikInfo*) (compressed_image.cc:394)
==29337==    by 0x1266D0: pik::PikToPixels(pik::DecompressParams const&, std::vector<unsigned char, std::allocator<unsigned char> > const&, pik::Image3<unsigned char>*, pik::PikInfo*) (pik.cc:544)
==29337==    by 0x10A89D: pik::(anonymous namespace)::Decompress(char const*, char const*) (dpik.cc:58)
==29337==    by 0x5DBC3F0: (below main) (libc-start.c:291)
==29337==
Pik check failed at opsin_codec.cc:886
==29337==
==29337== Process terminating with default action of signal 6 (SIGABRT)
==29337==    at 0x5DD177F: raise (raise.c:58)
==29337==    by 0x5DD3379: abort (abort.c:89)
==29337==    by 0x14CCDB: pik::DecodeImage(unsigned char const*, unsigned long, int, pik::Image3<short>*) (opsin_codec.cc:886)
==29337==    by 0x11CCE9: pik::CompressedImage::Decode(int, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, pik::PikInfo*) (compressed_image.cc:394)
==29337==    by 0x1266D0: pik::PikToPixels(pik::DecompressParams const&, std::vector<unsigned char, std::allocator<unsigned char> > const&, pik::Image3<unsigned char>*, pik::PikInfo*) (pik.cc:544)
==29337==    by 0x10A89D: pik::(anonymous namespace)::Decompress(char const*, char const*) (dpik.cc:58)
==29337==    by 0x5DBC3F0: (below main) (libc-start.c:291)
=================================================================
==10207==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffc4fdbcf40 at pc 0x000000657ee5 bp 0x7ffc4fdbc4b0 sp 0x7ffc4fdbc4a8
READ of size 4 at 0x7ffc4fdbcf40 thread T0
    #0 0x657ee4 in pik::BitReader::BitReader(unsigned char const*, unsigned long) /root/gwanyeong/pik/./bit_reader.h:36:36
    #1 0x657ee4 in pik::DecodeImageData(unsigned char const*, unsigned long, std::vector<unsigned char, std::allocator<unsigned char> > const&, int, pik::ANSSymbolReader*, pik::Image3<short>*) /root/gwanyeong/pik/opsin_codec.cc:744
    #2 0x65c319 in pik::DecodeImage(unsigned char const*, unsigned long, int, pik::Image3<short>*) /root/gwanyeong/pik/opsin_codec.cc:884:10
    #3 0x581268 in pik::CompressedImage::Decode(int, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, pik::PikInfo*) /root/gwanyeong/pik/compressed_image.cc:394:10
    #4 0x5afa67 in pik::PikToPixels(pik::DecompressParams const&, std::vector<unsigned char, std::allocator<unsigned char> > const&, pik::Image3<unsigned char>*, pik::PikInfo*) /root/gwanyeong/pik/pik.cc:543:29
    #5 0x50f124 in pik::(anonymous namespace)::Decompress(char const*, char const*) /root/gwanyeong/pik/dpik.cc:58:7
    #6 0x50f124 in main /root/gwanyeong/pik/dpik.cc:80
    #7 0x7f538d2d23f0 in __libc_start_main /build/glibc-mXZSwJ/glibc-2.24/csu/../csu/libc-start.c:291
    #8 0x41b759 in _start (/root/gwanyeong/pik/bin/dpik+0x41b759)

Address 0x7ffc4fdbcf40 is located in stack of thread T0 at offset 160 in frame
    #0 0x5af4df in pik::PikToPixels(pik::DecompressParams const&, std::vector<unsigned char, std::allocator<unsigned char> > const&, pik::Image3<unsigned char>*, pik::PikInfo*) /root/gwanyeong/pik/pik.cc:525

  This frame has 4 object(s):
    [32, 48) 'header'
    [64, 96) 'source'
    [128, 160) 'encoded_img' <== Memory access at offset 160 overflows this variable
    [192, 744) 'img'
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow /root/gwanyeong/pik/./bit_reader.h:36:36 in pik::BitReader::BitReader(unsigned char const*, unsigned long)
Shadow bytes around the buggy address:
  0x100009faf990: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100009faf9a0: 00 00 00 00 00 00 00 00 00 00 00 00 f3 f3 f3 f3
  0x100009faf9b0: f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 00 00 00 00
  0x100009faf9c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100009faf9d0: 00 00 00 00 f1 f1 f1 f1 00 00 f2 f2 00 00 00 00
=>0x100009faf9e0: f2 f2 f2 f2 00 00 00 00[f2]f2 f2 f2 00 00 00 00
  0x100009faf9f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100009fafa00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100009fafa10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100009fafa20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x100009fafa30: 00 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==10207==ABORTING
jan-wassenberg commented 7 years ago

Confirmed. Nice small test case, thanks!