#11344: av_clip functions, use of uninitialised value
-------------------------------------+-------------------------------------
Reporter: | Owner: (none)
names_are_hard |
Type: defect | Status: new
Priority: normal | Component:
| swresample
Version: git-master | Resolution:
Keywords: | Blocked By:
Blocking: | Reproduced by developer: 0
Analyzed by developer: 0 |
-------------------------------------+-------------------------------------
Description changed by names_are_hard:
Old description:
> 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.
>
> 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.
New description:
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.
--
--
Ticket URL: <https://trac.ffmpeg.org/ticket/11344#comment:1>
FFmpeg <https://ffmpeg.org>
FFmpeg issue tracker
_______________________________________________
FFmpeg-trac mailing list
FFmpeg-trac@avcodec.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-trac
To unsubscribe, visit link above, or email
ffmpeg-trac-requ...@ffmpeg.org with subject "unsubscribe".
