[FFmpeg-devel] [PATCH v2 7/7] avcodec: add AV_CODEC_FLAG_CLEAR

[Marton Balint]

On Thu, 7 Dec 2023, Anton Khirnov wrote:

> Quoting Ronald S. Bultje (2023-12-07 02:44:36)
>> Hi,
>> 
>> On Wed, Dec 6, 2023 at 3:23 AM Marton Balint <cus at passwd.hu> wrote:
>> 
>> > Signed-off-by: Marton Balint <cus at passwd.hu>
>> > ---
>> >  doc/APIchanges             |  3 +++
>> >  doc/codecs.texi            | 14 ++++++++++++++
>> >  libavcodec/avcodec.h       |  4 ++++
>> >  libavcodec/decode.c        |  6 ++++++
>> >  libavcodec/options_table.h |  1 +
>> >  libavcodec/version.h       |  2 +-
>> >  6 files changed, 29 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/doc/APIchanges b/doc/APIchanges
>> > index 416e2bec5e..f839504a64 100644
>> > --- a/doc/APIchanges
>> > +++ b/doc/APIchanges
>> > @@ -2,6 +2,9 @@ The last version increases of all libraries were on
>> > 2023-02-09
>> >
>> >  API changes, most recent first:
>> >
>> > +2023-12-xx - xxxxxxxxxxx - lavc 60.36.100 - avcodec.h
>> > +  Add AV_CODEC_FLAG_CLEAR.
>> > +
>> >  2023-12-xx - xxxxxxxxxxx - lavu 58.33.100 - imgutils.h
>> >    Add av_image_fill_color()
>> >
>> > diff --git a/doc/codecs.texi b/doc/codecs.texi
>> > index 5b950b4560..0504a535f2 100644
>> > --- a/doc/codecs.texi
>> > +++ b/doc/codecs.texi
>> > @@ -76,6 +76,20 @@ Apply interlaced motion estimation.
>> >  Use closed gop.
>> >  @item output_corrupt
>> >  Output even potentially corrupted frames.
>> > + at item clear
>> > +Clear the contents of the video buffer before decoding the next picture
>> > to it.
>> > +
>> > +Usually if only a part of a picture is affected by a decode error then the
>> > +decoder (if it implements error concealment) tries to hide it by
>> > interpolating
>> > +pixels from neighbouring areas or in some cases from the previous frame.
>> > Even
>> > +without error concealment it is quite likely that the affected area will
>> > +contain pixels from an earlier frame, due to frame pooling.
>> >
>> 
>> No comment on the patch itself, but wouldn't our users (and the C standard
>> itself) consider it a security issue to return stale
>
> I don't see the security issue in returning previously-returned frame
> data.

I guess what Ronald means that it is possible that the decoder frame 
pool allocates data in heap previously containing sensitive data, 
and that might never get overwritten in case of faulty input before 
passing it to the user.

The simple fix for that is to clear frame pool buffers on creation?

I am not sure if it is actually UB to read uninitialzied data from the 
heap though.

Regards,
Marton