video: vo_tct: lack check of mpi in draw_image

[void0red]

TL;DR

we perform a fuzz on mpv and find this bug.
here lacks a null ptr check on mpi, and dereference it directly, then it will crash.

mpv/video/out/vo_tct.c

Line 248 in e7902eb

struct mp_image src = *mpi;

Log file

AddressSanitizer:DEADLYSIGNAL
=================================================================
==6999==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7ffff55c2901 bp 0x7fffe6bf7a90 sp 0x7fffe6bf7258 T5)
==6999==The signal is caused by a READ memory access.
==6999==Hint: address points to the zero page.
    #0 0x7ffff55c2901 in memcpy (/lib/x86_64-linux-gnu/libc.so.6+0xc4901) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d)
    #1 0x555555735391 in __asan_memcpy (/root/mpv/build/mpv+0x1e1391) (BuildId: 738ec364ee9ba6016446014446b21f3245ca10c0)
    #2 0x555555b705bd in draw_image /root/mpv/build/../video/out/vo_tct.c:248:27
    #3 0x555555b5d0d0 in render_frame /root/mpv/build/../video/out/vo.c:971:13
    #4 0x555555b5d0d0 in vo_thread /root/mpv/build/../video/out/vo.c:1114:24
    #5 0x7ffff5592b42  (/lib/x86_64-linux-gnu/libc.so.6+0x94b42) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d)
    #6 0x7ffff5623bb3 in __clone (/lib/x86_64-linux-gnu/libc.so.6+0x125bb3) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/lib/x86_64-linux-gnu/libc.so.6+0xc4901) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d) in memcpy
Thread T5 created by T0 here:
    #0 0x55555571f3fc in pthread_create (/root/mpv/build/mpv+0x1cb3fc) (BuildId: 738ec364ee9ba6016446014446b21f3245ca10c0)
    #1 0x555555b53bed in vo_create /root/mpv/build/../video/out/vo.c:340:9
    #2 0x555555b52d4e in init_best_video_out /root/mpv/build/../video/out/vo.c:365:18
    #3 0x555555a22afc in reinit_video_chain_src /root/mpv/build/../player/video.c:232:28
    #4 0x5555559f2d06 in play_current_file /root/mpv/build/../player/loadfile.c:1706:5
    #5 0x5555559f2d06 in mp_play_files /root/mpv/build/../player/loadfile.c:1965:13
    #6 0x5555559fcca4 in mpv_main /root/mpv/build/../player/main.c:438:9
    #7 0x7ffff5527d8f  (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d)

==6999==ABORTING

[N-R-K]

Can you share the sample file that triggers this?

[void0red]

we perform 'software fault injection' on mpv, so sadly we can't provide a sample file, since the bug occurs when system oom.

mp_image_new_ref may return null when av_buffer_ref fails.

mpv/video/mp_image.c

Line 320 in e7902eb

*dst = av_buffer_ref(*dst);

so we need to check nullptr at draw_image, by the way, not only in tct out but also in some other draw_image

cmdline:

./build/mpv --vo=tct /root/output1.mp4

[philipl]

Huh. I would have expected it to abort explicitly on failed allocation. That's how talloc works, but I guess this is one of those friction points because of the use of av_buffer_ref.

[N-R-K]

we perform 'software fault injection' on mpv, so sadly we can't provide a sample file

I see.

by the way, not only in tct out but also in some other draw_image

Some of the other VO treat null image as a signal to clear the screen, it seems.

But if it's returning null due to av_buffer_ref() failing, then that seems like something that should be handled by the caller instead of being handled in draw_image.

[philipl]

This is generally odd. Some callers of mp_image_new_ref do MP_HANDLE_OOM on the return value, which is what I'd expect. A couple explicitly check for NULL, and others return back up the call chain to who knows what. I'm surprised that the MP_HANDLE_OOM is not inside mp_image_new_ref, which is the fix I would recommend. Then it's very clear that we'll abort on a failed allocation. There's no value in any of the callers handling it gracefully.