google / pik

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

stack-buffer-overflow in pik::HuffmanDecodingData::ReadFromBitStream #17

Closed gy741 closed 7 years ago

gy741 commented 7 years ago

Hi.

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

Please confirm.

Thanks.

Summary: stack-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


=================================================================
==30122==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffc74c2fb20 at pc 0x0000005d3f0e bp 0x7ffc74c2f1b0 sp 0x7ffc74c2f1a8
READ of size 4 at 0x7ffc74c2fb20 thread T0
    #0 0x5d3f0d in pik::BitReader::FillBitBuffer() /root/gwanyeong/pik/./bit_reader.h:46:39
    #1 0x5d3f0d in pik::BitReader::ReadBits(int) /root/gwanyeong/pik/./bit_reader.h:68
    #2 0x5d3f0d in pik::HuffmanDecodingData::ReadFromBitStream(pik::BitReader*) /root/gwanyeong/pik/huffman_decode.cc:269
    #3 0x660cce in pik::DecodePlane(unsigned char const*, unsigned long, int, int, pik::Image<int>*) /root/gwanyeong/pik/opsin_codec.cc:992:8
    #4 0x6a0578 in pik::Quantizer::Decode(unsigned char const*, unsigned long) /root/gwanyeong/pik/quantizer.cc:141:10
    #5 0x581212 in pik::CompressedImage::DecodeQuantization(unsigned char const*, unsigned long) /root/gwanyeong/pik/compressed_image.cc:380:21
    #6 0x581212 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:393
    #7 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
    #8 0x50f124 in pik::(anonymous namespace)::Decompress(char const*, char const*) /root/gwanyeong/pik/dpik.cc:58:7
    #9 0x50f124 in main /root/gwanyeong/pik/dpik.cc:80
    #10 0x7fabb4e083f0 in __libc_start_main /build/glibc-mXZSwJ/glibc-2.24/csu/../csu/libc-start.c:291
    #11 0x41b759 in _start (/root/gwanyeong/pik/bin/dpik+0x41b759)

Address 0x7ffc74c2fb20 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:46:39 in pik::BitReader::FillBitBuffer()
Shadow bytes around the buggy address:
  0x10000e97df10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10000e97df20: 00 00 00 00 00 00 00 00 f3 f3 f3 f3 f3 f3 f3 f3
  0x10000e97df30: f3 f3 f3 f3 f3 f3 f3 f3 00 00 00 00 00 00 00 00
  0x10000e97df40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10000e97df50: f1 f1 f1 f1 00 00 f2 f2 00 00 00 00 f2 f2 f2 f2
=>0x10000e97df60: 00 00 00 00[f2]f2 f2 f2 00 00 00 00 00 00 00 00
  0x10000e97df70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10000e97df80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10000e97df90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10000e97dfa0: 00 00 00 00 00 00 00 00 00 00 00 00 00 f3 f3 f3
  0x10000e97dfb0: f3 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
==30122==ABORTING
jan-wassenberg commented 7 years ago

Also confirmed, thanks!

szabadka commented 7 years ago

This is fixed in the latest version.