Opened 22 hours ago

Last modified 22 hours ago

#11344 new defect

av_clip functions, use of uninitialised value

Reported by: names_are_hard Owned by:
Priority: normal Component: swresample
Version: git-master Keywords:
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description (last modified by names_are_hard)

Some av_clip* functions, including at least av_clip_uintp2_c() and av_clip_int16_c(), have behaviour dependent on uninitialised values due to use in a conditional.

I have guessed the component to be swresample based on file names in valgrind trace, my confidence is moderate.

Samples are attached, if Trac will let me.

Repro instructions:
valgrind --track-origins=yes ffmpeg_g -i some_sample -nostdin -f null -

Tested against master @ 1e76bd2f39.

Truncated example valgrind output:

==882956== Conditional jump or move depends on uninitialised value(s)
==882956==    at 0x118EA3E: av_clip_int16_c (common.h:245)
==882956==    by 0x118EA3E: conv_AV_SAMPLE_FMT_FLT_to_AV_SAMPLE_FMT_S16 (audioconvert.c:79)
==882956==    by 0x118EEDB: swri_audio_convert (audioconvert.c:248)
[...]
==882956==  Uninitialised value was created by a heap allocation
==882956==    at 0x4848DA0: memalign (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
[...]
==882956==    by 0x8CCD08: audio_get_buffer (get_buffer.c:193)
==882956==    by 0x8CCD08: avcodec_default_get_buffer2 (get_buffer.c:284)

The specific fix should probably find why initialisation is skipped and fix the root cause. This is hard for me to determine, as I have little experience with ffmpeg. Quite possibly, more av_clip* functions can be reached in a similar way, with a different input, given the similarity in traces for the two examples provided here.

A more general fix that I would recommend also employing is to use calloc() where possible when allocating structs / object-like entities. On "large" OSes, this is likely to have no performance impact, as the OS will keep a pool of zero pages available for this purpose. If there are concerns about perf on smaller OSes, a malloc-like function could be chosen at build time for these targets.

This would reduce control available to attackers, and make coverage guided fuzzing more efficient. Currently, ffmpeg coverage is non-deterministic on many inputs due to this and similar bugs.

Attachments (1)

av_clip_samples.zip (2.0 KB ) - added by names_are_hard 22 hours ago.

Download all attachments as: .zip

Change History (2)

by names_are_hard, 22 hours ago

Attachment: av_clip_samples.zip added

comment:1 by names_are_hard, 22 hours ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.