[FFmpeg-devel] [PATCH] lavc/cbs_h265_syntax_template: Fix memleak

[Andreas Rheinhardt]

Andriy Gelman:
> From: Andriy Gelman <andriy.gelman at gmail.com>
> 
> payload_count is used to track the number of SEI payloads. It is also
> used to free the SEIs in cbs_h265_free_sei().
> 
> Currently, payload_count is set after for loop is completed. Hence if
> there is an error and the function exits, the payload_remains zero
> causing a memleak.
> 
> This commit keeps track of payload_count inside the for loop to fix the
> issue. Note that that the contents of current are initialized with
> av_mallocz so there is no need to zero initialize payload_count.
> 
> Found-by: libFuzzer
> Signed-off-by: Andriy Gelman <andriy.gelman at gmail.com>
> ---
>  libavcodec/cbs_h265_syntax_template.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/cbs_h265_syntax_template.c b/libavcodec/cbs_h265_syntax_template.c
> index 54570929ec7..57fb2f3aa6a 100644
> --- a/libavcodec/cbs_h265_syntax_template.c
> +++ b/libavcodec/cbs_h265_syntax_template.c
> @@ -2167,6 +2167,7 @@ static int FUNC(sei)(CodedBitstreamContext *ctx, RWContext *rw,
>          uint32_t payload_size = 0;
>          uint32_t tmp;
>  
> +        current->payload_count++;
>          while (show_bits(rw, 8) == 0xff) {
>              fixed(8, ff_byte, 0xff);
>              payload_type += 255;
> @@ -2194,7 +2195,6 @@ static int FUNC(sei)(CodedBitstreamContext *ctx, RWContext *rw,
>                 "SEI message: found %d.\n", k);
>          return AVERROR_INVALIDDATA;
>      }
> -    current->payload_count = k + 1;
>  #else
>      for (k = 0; k < current->payload_count; k++) {
>          PutBitContext start_state;
> 
Good catch. But you should only increment payload_count directly
before CHECK(FUNC(sei_payload)(ctx, rw, &current->payload[k], prefix))
-- there are after all no allocations performed before it.
And cbs_h264 suffers from the same problem; it should probably be in
the same commit.

- Andreas