Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OOB Read/Write in HeifPixelImage::overlay() #1226

Closed
flyyee opened this issue Jul 7, 2024 · 2 comments
Closed

OOB Read/Write in HeifPixelImage::overlay() #1226

flyyee opened this issue Jul 7, 2024 · 2 comments

Comments

@flyyee
Copy link
Contributor

flyyee commented Jul 7, 2024

Overview

Due to insufficient validation of image overlay offset values, it is possible to OOB read & write in HeifPixelImage::overlay().

Reproduction

This vulnerability exists on both master (7c9729e) and develop-v1.18.0 (44a9705).

git clone https://github.com/strukturag/libheif.git
cd libheif
mkdir build && cd build
CC="gcc -fsanitize=address" CXX="g++ -fsanitize=address" cmake ..
make -j

# OOB read
examples/heif-info poc_oob_read.heic

# OOB write
examples/heif-info poc_oob_write.heic

Download from google drive:
poc_oob_read.heic
poc_oob_write.heic

Asan logs

OOB read:

==169466==ERROR: AddressSanitizer: negative-size-param: (size=-2147482208)
    #0 0x7ffff76053ff in __interceptor_memcpy ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827
    #1 0x7ffff73b61e0 in HeifPixelImage::overlay(std::shared_ptr<HeifPixelImage>&, int, int) (clean_build/libheif/build/libheif/libheif.so.1+0x1b61e0)
    #2 0x7ffff733cabc in HeifContext::decode_overlay_image(unsigned int, std::shared_ptr<HeifPixelImage>&, std::vector<unsigned char, std::allocator<unsigned char> > const&, heif_decoding_options const&) const (clean_build/libheif/build/libheif/libheif.so.1+0x13cabc)
    #3 0x7ffff7333f60 in HeifContext::decode_image_planar(unsigned int, std::shared_ptr<HeifPixelImage>&, heif_colorspace, heif_decoding_options const&, bool) const (clean_build/libheif/build/libheif/libheif.so.1+0x133f60)
    #4 0x7ffff7331450 in HeifContext::decode_image_user(unsigned int, std::shared_ptr<HeifPixelImage>&, heif_colorspace, heif_chroma, heif_decoding_options const&) const (clean_build/libheif/build/libheif/libheif.so.1+0x131450)
    #5 0x7ffff72eb0d9 in heif_decode_image (clean_build/libheif/build/libheif/libheif.so.1+0xeb0d9)
    #6 0x5555555625f2 in main (clean_build/libheif/build/examples/heif-info+0xe5f2)
    #7 0x7ffff6ccdd8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #8 0x7ffff6ccde3f in __libc_start_main_impl ../csu/libc-start.c:392
    #9 0x55555555c304 in _start (clean_build/libheif/build/examples/heif-info+0x8304)

OOB write:

==169585==ERROR: AddressSanitizer: SEGV on unknown address 0x62e080000400 (pc 0x7ffff6d68ae3 bp 0x7fffffffbc00 sp 0x7fffffffb9b8 T0)
==169585==The signal is caused by a WRITE memory access.
    #0 0x7ffff6d68ae3  (/lib/x86_64-linux-gnu/libc.so.6+0xc4ae3)
    #1 0x7ffff73b61e0 in HeifPixelImage::overlay(std::shared_ptr<HeifPixelImage>&, int, int) (clean_build/libheif/build/libheif/libheif.so.1+0x1b61e0)
    #2 0x7ffff733cabc in HeifContext::decode_overlay_image(unsigned int, std::shared_ptr<HeifPixelImage>&, std::vector<unsigned char, std::allocator<unsigned char> > const&, heif_decoding_options const&) const (clean_build/libheif/build/libheif/libheif.so.1+0x13cabc)
    #3 0x7ffff7333f60 in HeifContext::decode_image_planar(unsigned int, std::shared_ptr<HeifPixelImage>&, heif_colorspace, heif_decoding_options const&, bool) const (clean_build/libheif/build/libheif/libheif.so.1+0x133f60)
    #4 0x7ffff7331450 in HeifContext::decode_image_user(unsigned int, std::shared_ptr<HeifPixelImage>&, heif_colorspace, heif_chroma, heif_decoding_options const&) const (clean_build/libheif/build/libheif/libheif.so.1+0x131450)
    #5 0x7ffff72eb0d9 in heif_decode_image (clean_build/libheif/build/libheif/libheif.so.1+0xeb0d9)
    #6 0x5555555625f2 in main (clean_build/libheif/build/examples/heif-info+0xe5f2)
    #7 0x7ffff6ccdd8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #8 0x7ffff6ccde3f in __libc_start_main_impl ../csu/libc-start.c:392
    #9 0x55555555c304 in _start (clean_build/libheif/build/examples/heif-info+0x8304)

Environment

OS: Ubuntu 22.04
gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0
g++ (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0

Root cause

The overlay offsets specified in the iloc box are trusted by the program. While there are "sanity" checks in HeifPixelImage::overlay(), these checks can be bypassed with extremely large or extremely small offsets.

For instance, in poc_oob_write.heic, the x offset (dx) is INT_MAX - 10. The OOB write arises from an integer overflow in this check:

    // overlay image extends past the right border -> cut width for copy
    if (dx + in_w > out_w) {
      in_w = out_w - dx;
    }

In poc_oob_read.heic, dx is INT_MIN. The OOB read arises from the UB in this check:

    // overlay image started outside of left border
    // -> move start into the image and start at left output column
    if (dx < 0) {
      in_x0 = -dx;
      out_x0 = 0;
    }

Mitigation

A simple fix would be to check that in_y0/x0 and out_x0/y0 are within the bounds of the respective images' dimensions before using the values at the end of the function.

@flyyee
Copy link
Contributor Author

flyyee commented Jul 7, 2024

Opened a PR with the suggested fix: #1227

@farindk
Copy link
Contributor

farindk commented Jul 8, 2024

I have rewritten the whole section. Should now handle all edge cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants