Heap-buffer-overflow in fallback-motion.cc: void put_epel_hv_fallback<unsigned short>(

[fdu-sec]

Description

Heap-buffer-overflow (/libde265/build/libde265/liblibde265.so+0x148fda) in void put_epel_hv_fallback(short*, long, unsigned short const*, long, int, int, int, int, short*, int)

Version

$ ./dec265 -h
 dec265  v1.0.8
--------------
usage: dec265 [options] videofile.bin
The video file must be a raw bitstream, or a stream with NAL units (option -n).

options:
  -q, --quiet       do not show decoded image
  -t, --threads N   set number of worker threads (0 - no threading)
  -c, --check-hash  perform hash check
  -n, --nal         input is a stream with 4-byte length prefixed NAL units
  -f, --frames N    set number of frames to process
  -o, --output      write YUV reconstruction
  -d, --dump        dump headers
  -0, --noaccel     do not use any accelerated code (SSE)
  -v, --verbose     increase verbosity level (up to 3 times)
  -L, --no-logging  disable logging
  -B, --write-bytestream FILENAME  write raw bytestream (from NAL input)
  -m, --measure YUV compute PSNRs relative to reference YUV
  -T, --highest-TID select highest temporal sublayer to decode
      --disable-deblocking   disable deblocking filter
      --disable-sao          disable sample-adaptive offset filter
  -h, --help        show help

Replay

git clone https://github.com/strukturag/libde265.git
cd libde265
mkdir build
cd build
cmake ../ -DCMAKE_CXX_FLAGS="-fsanitize=address"
make -j$(nproc)
./dec265/dec265 poc11-1
./dec265/dec265 poc11-2

ASAN

WARNING: pps header invalid
WARNING: CTB outside of image area (concealing stream error...)
WARNING: CTB outside of image area (concealing stream error...)
=================================================================
==61372==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62b00002951c at pc 0x7f3e99904fdb bp 0x7ffe34d063b0 sp 0x7ffe34d063a0
READ of size 2 at 0x62b00002951c thread T0
    #0 0x7f3e99904fda in void put_epel_hv_fallback<unsigned short>(short*, long, unsigned short const*, long, int, int, int, int, short*, int) (/libde265/build/libde265/liblibde265.so+0x148fda)
    #1 0x7f3e999332ca in acceleration_functions::put_hevc_epel_hv(short*, long, void const*, long, int, int, int, int, short*, int) const (/libde265/build/libde265/liblibde265.so+0x1772ca)
    #2 0x7f3e99935213 in void mc_chroma<unsigned short>(base_context const*, seq_parameter_set const*, int, int, int, int, short*, int, unsigned short const*, int, int, int, int) (/libde265/build/libde265/liblibde265.so+0x179213)
    #3 0x7f3e99925b2d in generate_inter_prediction_samples(base_context*, slice_segment_header const*, de265_image*, int, int, int, int, int, int, int, PBMotion const*) (/libde265/build/libde265/liblibde265.so+0x169b2d)
    #4 0x7f3e9993290f in decode_prediction_unit(base_context*, slice_segment_header const*, de265_image*, PBMotionCoding const&, int, int, int, int, int, int, int, int) (/libde265/build/libde265/liblibde265.so+0x17690f)
    #5 0x7f3e9996d7e3 in read_prediction_unit(thread_context*, int, int, int, int, int, int, int, int, int) (/libde265/build/libde265/liblibde265.so+0x1b17e3)
    #6 0x7f3e9996f39a in read_coding_unit(thread_context*, int, int, int, int) (/libde265/build/libde265/liblibde265.so+0x1b339a)
    #7 0x7f3e99970250 in read_coding_quadtree(thread_context*, int, int, int, int) (/libde265/build/libde265/liblibde265.so+0x1b4250)
    #8 0x7f3e99970091 in read_coding_quadtree(thread_context*, int, int, int, int) (/libde265/build/libde265/liblibde265.so+0x1b4091)
    #9 0x7f3e99967726 in read_coding_tree_unit(thread_context*) (/libde265/build/libde265/liblibde265.so+0x1ab726)
    #10 0x7f3e999709ea in decode_substream(thread_context*, bool, bool) (/libde265/build/libde265/liblibde265.so+0x1b49ea)
    #11 0x7f3e9997270f in read_slice_segment_data(thread_context*) (/libde265/build/libde265/liblibde265.so+0x1b670f)
    #12 0x7f3e998d16d2 in decoder_context::decode_slice_unit_sequential(image_unit*, slice_unit*) (/libde265/build/libde265/liblibde265.so+0x1156d2)
    #13 0x7f3e998d1ec1 in decoder_context::decode_slice_unit_parallel(image_unit*, slice_unit*) (/libde265/build/libde265/liblibde265.so+0x115ec1)
    #14 0x7f3e998d0c0f in decoder_context::decode_some(bool*) (/libde265/build/libde265/liblibde265.so+0x114c0f)
    #15 0x7f3e998d093d in decoder_context::read_slice_NAL(bitreader&, NAL_unit*, nal_header&) (/libde265/build/libde265/liblibde265.so+0x11493d)
    #16 0x7f3e998d343e in decoder_context::decode_NAL(NAL_unit*) (/libde265/build/libde265/liblibde265.so+0x11743e)
    #17 0x7f3e998d3ab3 in decoder_context::decode(int*) (/libde265/build/libde265/liblibde265.so+0x117ab3)
    #18 0x7f3e998bae95 in de265_decode (/libde265/build/libde265/liblibde265.so+0xfee95)
    #19 0x55a40ac18bc9 in main (/libde265/build/dec265/dec265+0x6bc9)
    #20 0x7f3e993ecc86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)
    #21 0x55a40ac169b9 in _start (/libde265/build/dec265/dec265+0x49b9)

0x62b00002951c is located 12 bytes to the right of 25360-byte region [0x62b000023200,0x62b000029510)
allocated by thread T0 here:
    #0 0x7f3e99de3790 in posix_memalign (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdf790)
    #1 0x7f3e9990c1cb in ALLOC_ALIGNED(unsigned long, unsigned long) (/libde265/build/libde265/liblibde265.so+0x1501cb)
    #2 0x7f3e9990c99d in de265_image_get_buffer(void*, de265_image_spec*, de265_image*, void*) (/libde265/build/libde265/liblibde265.so+0x15099d)
    #3 0x7f3e9990ed1a in de265_image::alloc_image(int, int, de265_chroma, std::shared_ptr<seq_parameter_set const>, bool, decoder_context*, long, void*, bool) (/libde265/build/libde265/liblibde265.so+0x152d1a)
    #4 0x7f3e998f30cc in decoded_picture_buffer::new_image(std::shared_ptr<seq_parameter_set const>, decoder_context*, long, void*, bool) (/libde265/build/libde265/liblibde265.so+0x1370cc)
    #5 0x7f3e998d4824 in decoder_context::generate_unavailable_reference_picture(seq_parameter_set const*, int, bool) (/libde265/build/libde265/liblibde265.so+0x118824)
    #6 0x7f3e998d7332 in decoder_context::process_reference_picture_set(slice_segment_header*) (/libde265/build/libde265/liblibde265.so+0x11b332)
    #7 0x7f3e998dad70 in decoder_context::process_slice_segment_header(slice_segment_header*, de265_error*, long, nal_header*, void*) (/libde265/build/libde265/liblibde265.so+0x11ed70)
    #8 0x7f3e998d0246 in decoder_context::read_slice_NAL(bitreader&, NAL_unit*, nal_header&) (/libde265/build/libde265/liblibde265.so+0x114246)
    #9 0x7f3e998d343e in decoder_context::decode_NAL(NAL_unit*) (/libde265/build/libde265/liblibde265.so+0x11743e)
    #10 0x7f3e998d3ab3 in decoder_context::decode(int*) (/libde265/build/libde265/liblibde265.so+0x117ab3)
    #11 0x7f3e998bae95 in de265_decode (/libde265/build/libde265/liblibde265.so+0xfee95)
    #12 0x55a40ac18bc9 in main (/libde265/build/dec265/dec265+0x6bc9)
    #13 0x7f3e993ecc86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86)

SUMMARY: AddressSanitizer: heap-buffer-overflow (/libde265/build/libde265/liblibde265.so+0x148fda) in void put_epel_hv_fallback<unsigned short>(short*, long, unsigned short const*, long, int, int, int, int, short*, int)
Shadow bytes around the buggy address:
  0x0c567fffd250: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c567fffd260: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c567fffd270: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c567fffd280: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c567fffd290: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c567fffd2a0: 00 00 fa[fa]fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c567fffd2b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c567fffd2c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c567fffd2d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c567fffd2e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c567fffd2f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
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
==61372==ABORTING

POC

https://github.com/FDU-Sec/poc/blob/main/libde265/poc11-1
https://github.com/FDU-Sec/poc/blob/main/libde265/poc11-2

Environment

Ubuntu 16.04
Clang 10.0.1
gcc 5.5

Credit

Peng Deng (Fudan University)

[coldtobi]

Hi

(in advance, sorry for the wall of text… I'm just trying to convey as much information as
possible and as my knowledge on h.265 is limited, I try to be as verbose
as possible so that errors in my thinking are more obviouls caught…)

I was debugging into this crash and like to share my findings.
Unfortunatly the h265 standard is too complicated for me to grasp
what the right thing would be to do…

I've using a lots of additional printfs for debugging output, the branch I'm using
is this one:
coldtobi@c4c3a3a

I've compiled it on Debian unstable with asan, cmake invocation:
cmake ../libde265 -DCMAKE_CXX_FLAGS="-fsanitize=address" -DCMAKE_BUILD_TYPE=Debug

The complete log of poc11-1 is here (using my debug code)

• without -d: libdec265-poc11-1.txt
• with -d: libdec265-poc11-1-withdump.txt

TL;DR: I think (not sure if I read the standard correctly here) that
the poc is sending a new SPS during a VCL, which would be not allowed as 7.4.2.4.2.

This tricks the decoder to mix different images generated with different SPS is the same decoded pictures,
leading to the seen out-of-bound memory accesses.

How to Fix?
I'm not sure how to fix that without breaking compatiblity with the standard,
-- frankly I wouldn't be able to recognize this…

Said that here I've got an fix to at least avoid the crashes, by
rejecting reference pictures if they are created by a different SPS.

One way would be just to look at the pointers of the SPS (fast and easy, but
may reject more than required),
or investigate if the SPS used for the image generations are "compatible".
[A] below could help for that.
The Standard C3.2 the paragraph starting
with "Otherwise, if the value of" seems to have a list of parameters that
needs to be matched, but I'm not sure if that list is sensible/complete…

(I'll submit as PR later, after cleaning it up, to have a better base
to discuss that.)

Some other observations:

• [A] It seems that the standard allows (active) SPS, VPS and PPS to be re-transmitted,
  -- if I read 7.4.2.4.2; page 69 last pargraph correctly -- during an active
  VCS, but then it has to have the same content.
  The idea "reject any reference images if they have not been created the image's sps"
  might benefit from checking if a newly received SPS is identical to the one it
  would replace.

• in the sequence leading up to the asan failure, I've observed that bascially all
  calls in the chain (e.g decode_substream(), read_coding_tree_unit(),
  read_coding_quadtree(), read_coding_unit(), decode_quantization_parameters()) leading
  to the asan-triggering generate_inter_prediction_samples()
  is getting the sps and pps pointer from the image. In contrast, the function
  generate_inter_prediction_samples() extracts it from the shdr.
  I've seen in another poc (unfortuantly did not note which one) that this leaded
  to a situation where the wrong sps was used, in the meaning that the sps's parameters
  did not match the parameters of the image. Changing that fixed several crashes,
  I'll try to follow up on that seperatly.

** verbose debug log**

I believe the poc11-1 does this evil:

(- Generating VPS #1) (in log: in memory at 0x617000000090)

• Generating SPS #1 (with seq_parameter_set_id=0 and bit_depth_chroma=8) (in memory at 0x629000000210)
  (- Generating PPS #1 ) pic_parameter_set_id=0 -- in memory at 0x625000002910)

• slice #1 (SHDR at 0x61b000000780)

  • generates output image ID=0 with VPS#1 PPS#1 and SPS#1

• slice #2 (SHDR at 0x61b000000e80)

  • generates output image ID=1 with VPS#1 PPS#1 and SPS#1
  • generates (missing reference) image ID=2 SPS#1 for POC=-2

(- output of image #0)

• Generating SPS #2 (with seq_parameter_set_id=0 and bit_depth_chroma=9) -- memory location 0x629000005210
  This new SPS #2 replaces SPS #1 - (PPS#1 is removed from decoder_context::pps[])
  (- Generating PPS #2 with pic_parameter_set_id=0 , memory location #0x62500000c910)

• slice #3 (SHDR at 0x61b000001580)

  • generating image ID=3 with VPS#1 *SPS#2* PPS#2)
  • it seems that this image is referencing the image with ID=2.
    noting that this image has been created with SPS#1 so it has only a 8 bits chroma.
  • "remove ID 0" -- DPB[0] set to unused.

At this point the DPB contains:

 DPB  0: POC=0   , ID=  0 img=0x616000000080 short-term ------	(vps=0x617000000090 sps=0x629000000210 pps=0x625000002910)
 DPB  1: POC=-1  , ID=  1 img=0x616000000380 short-term output	(vps=0x617000000090 sps=0x629000000210 pps=0x625000002910)
 DPB  2: POC=-2  , ID=  2 img=0x616000000680 short-term ------	(vps=(nil) sps=0x629000000210 pps=(nil))
 DPB  3: POC=-3  , ID=  3 img=0x616000000980 short-term ------	(vps=0x617000000090 sps=0x629000005210 pps=0x62500000c910)

(- output of image #1)

• slice #4
  • generating image ID=4 with VPS#1 *SPS#2* PPS#2 (taking the place in dpb at dpb[0])
  • generating (missing reference) image ID=5 with VPS#1 *SPS#2* PPS#2 for POC=-8
  • generating (missing reference) image ID=6 with VPS#1 *SPS#2* PPS#2 for POC=-17
  • generating (missing reference) image ID=7 with VPS#1 *SPS#2* PPS#2 for POC=-23
  • generating (missing reference) image ID=8 with VPS#1 *SPS#2* PPS#2 for POC=0
  • "remove ID 1" -- DPB[1] set to unused.

Now do_some_work() is invoked, and image_units[0] is being handled:

• image_units[0] contains (taken from gdb, not in the logs):
  - image -> 0x616000000980 (image ID=3)
  - imgunit->get_next_unprocessed_slice_segment() returns slice #3 (0x61b000001580)

Taking a look at the SHDR, I can reconfirm that the reference set contains:

  shdr->RefPicList[2][MAX_NUM_REF_PICS] contains [2 , 2 , ( remaining 0s) ], [2 , 2 , ( remaining 0s) ]
  shdr->RefPicList_POC[2][MAX_NUM_REF_PICS] contains [-2 , -2 , ( remaining 0s) ], [-2 , -2 , ( remaining 0s) ]
  shdr->RefPicList_PicState[2][MAX_NUM_REF_PICS] contains [1 , 1 , ( remaining 0s) ], [1 , 1 , ( remaining 0s)
  shrs->LongTermRefPic[2][MAX_NUM_REF_PICS] is all zero.

- decoder_context::decode_slice_unit_parallel() is now started, the DPB is at this point of time (after
the call to remove_images_from_dpb(sliceunit->shdr->RemoveReferencesList) in decoder_context::decode_slice_unit_parallel())

  ####DPB start of decoder_context::decode_slice_unit_parallel()
 DPB  0: POC=-1  , ID=  4 img=0x616000000080 short-term output	(vps=0x617000000090 sps=0x629000005210 pps=0x62500000c910)
 DPB  1: POC=-1  , ID=  1 img=0x616000000380 unused     ------	(vps=0x617000000090 sps=0x629000000210 pps=0x625000002910)
 DPB  2: POC=-2  , ID=  2 img=0x616000000680 short-term ------	(vps=(nil) sps=0x629000000210 pps=(nil))
 DPB  3: POC=-3  , ID=  3 img=0x616000000980 short-term ------	(vps=0x617000000090 sps=0x629000005210 pps=0x62500000c910)
 DPB  4: POC=-8  , ID=  5 img=0x616000000c80 short-term ------	(vps=(nil) sps=0x629000005210 pps=(nil))
 DPB  5: POC=-17 , ID=  6 img=0x616000000f80 short-term ------	(vps=(nil) sps=0x629000005210 pps=(nil))
 DPB  6: POC=-23 , ID=  7 img=0x616000001280 short-term ------	(vps=(nil) sps=0x629000005210 pps=(nil))
 DPB  7: POC=0   , ID=  8 img=0x616000001580 short-term ------	(vps=(nil) sps=0x629000005210 pps=(nil))

• eventually, decoding will call generate_inter_prediction_samples(), and then hit my debugigng trap emitting
  this line:

#######################  TROUBLE AHEAD: REF PICTURE DIFFERS IS BITDEPTH ####################### 
 shdr=0x61b000001580 , img=0x616000000980 (ID=3), refPic=0x616000000680 (ID=2) refIdx: 1 -> dpb[2] refPic->BitDepth_C=8, img->BitDepth_C=9 refPic->BitDepth_Y=8, img->BitDepth_Y=8

• this confirms that image ID=3, which has been created with SPS#2 is using as reference image ID=2, which
  has been created with SPS#1 -- with different storage units in img->pixels[{1,2}];
• later in generate_inter_prediction_samples() the bit depth of the reference picture is used to
  determine the template parameter pixel_t, IOW which flavour or mc_chroma<pixel_t>() or mc_luma<pixel_t>() is to be called.
• as this function is doing pointer arithemtics, if RefPic's pixel is uint16_t and Img's pixel_t is uint8_t, it will
  eventually explode:
  (This is for the asan error that is triggered for poc11-1: )
  • function: put_epel_hv_fallback<uint16_t>(), , line: 353: case 5: v = (-4*p[0]+28*p[1]+46*p[2]-6*p[3])>>shift1; break;
  • p=0x62b00002951c

    • calculated from&src[y*src_stride - extra_left];
      with y=8, src_stride=176, extra-left=1 so it is &src[1408] (or 2814 bytes into the src)

    • caller has set src to 0x62b000028a1e, (so p=0x62b00002951c)

    • src is calculated in mc_chroma(): its template/pixel_t type is uint16_t.
src_ptr = &ref[xIntOffsC + yIntOffsC*ref_stride];
ref=refPic->get_image_plane(1) -> 0x62b000023200 (rePic->pixel[1] = 0x62b000023200; 25360 bytes in size)
      xIntOffsC=191, yIntOffsC=63, refStride=176 --> so src_ptr becomes
      - src_ptr = &ref[11279] (or as <pixel_t>=uint16_t 22556 bytes into rePic->pixel[1]
      - src_ptr = 0x62b000028a1e

    • the code tries to access 0x62b00002951c + (sizeof(uint16_t)*3) = 0x62b00002951c + 6 = 0x62b00002951c + 6 = 0x62b000029522

    • however, pixel[1] ends at 0x62b000023200 + 25360 = 0x62b000029510, so 0x62b00002951c is outside of the allocated area (as asan correctly reports:

           ==731779==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62b00002951c at pc 0x7ffff75622db bp 0x7ffffffdca20 sp 0x7ffffffdca18
            0x62b00002951c is located 12 bytes to the right of 25360-byte region [0x62b000023200,0x62b000029510)

[coldtobi]

libdec265-poc11-1-withdump.txt
PS: Those issues might be related; at least when rejecting "inappropiate" reference picutures (as in PR #365 ), the following POC stop failing:

• poc3 (Heap-buffer-overflow in sse-motion.cc: ff_hevc_put_hevc_epel_pixels_8_sse #337)
• poc7-1 (Heap-buffer-overflow in motion.cc: mc_chroma<unsigned short> #341) CVE-2022-43239 (note: does NOT fix poc7-2)
• poc8-2, poc8-3, poc8-4 (Heap-buffer-overflow in fallback-motion.cc: in void put_qpel_fallback<unsigned short> #342) CVE-2022-43244 (note: does NOT fix poc8-1)
• poc11-1, poc11-2 (Heap-buffer-overflow in fallback-motion.cc: void put_epel_hv_fallback<unsigned short>( #345) CVE-2022-43249
• poc12 (Heap-buffer-overflow in fallback-motion.cc: in put_qpel_0_0_fallback_16 #346)
• poc13 (Heap-buffer-overflow in fallback-motion.cc in put_epel_16_fallback #347) CVE-2022-43252
• poc16 (Stack-buffer-overflow in sao.cc:248: void apply_sao_internal<unsigned short> #350)

[coldtobi]

According to Debian this is CVE-2022-43249

[farindk]

Thanks for the extensive analysis and patch.
Since the actual cause is that reference image and current image have different bit depths, what about simply checking if they match (fbd0b3a)?

While your patch tries to prevent the situation by detecting it high-level, it is complicated to compare SPSs for "compatibility" and whether they are simply repeated (51f07f1). I think, this is very error prone. Checking for matching bit-depth directly, on the other hand, secures the function itself, which may also be helpful if used in another context. And it also allows us to output a warning what actually is wrong with the input stream.

There might be a performance penalty in carrying out this check for each PB, but here I simply hope for a good branch prediction.

[coldtobi]

what about simply checking if they match (fbd0b3a)?

This was my first approach, and while it fixes that specific exploit bitdepth is not the only paramter to be checked; for example I saw also crashes out of the poc if the dimensions where not matching the expectaions.
I'm pretty sure that there are many more, and I think my approach here is more robust than to individually check every different scenario… That would feel like a road to play whack-a-mole for me…

Comparing the sps is of course a "brute force" attempt to fix that;
For example (if I understood it correclty), sps updates are only allowed in certain cirmumstances, I read something in the standards.

The patch to check for compatiblity was only for to avoid over-rejecting. My reference videos played well without it, but I thought "better save here". This patch is also erroring on the "it's incompatible if in doubt" side.

[farindk]

Should be fixed with #373