On Sat, Jun 24, 2023 at 10:13:48PM +0200, Reimar Döffinger wrote:
> Hi!
>
> > On 24 Jun 2023, at 21:14, Andreas Rheinhardt
> > <andreas.rheinha...@outlook.com> wrote:
> >
> > Michael Niedermayer:
> >> Fixes: out of array access
> >> Fixes: crash-0d640731c7da52415670eb47a2af701cbe2e1a3b
> >>
> >> Found-by: Catena cyber <cont...@catenacyber.fr>
> >> Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc>
> >> ---
> >> libavcodec/parser.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/libavcodec/parser.c b/libavcodec/parser.c
> >> index efc28b8918..db39e698ab 100644
> >> --- a/libavcodec/parser.c
> >> +++ b/libavcodec/parser.c
> >> @@ -214,7 +214,7 @@ int ff_combine_frame(ParseContext *pc, int next,
> >> for (; pc->overread > 0; pc->overread--)
> >> pc->buffer[pc->index++] = pc->buffer[pc->overread_index++];
> >>
> >> - if (next > *buf_size)
> >> + if (next > *buf_size || (next < -pc->index && next != END_NOT_FOUND))
> >> return AVERROR(EINVAL);
> >>
> >> /* flush remaining if EOF */
> >
> > Could you provide more details about this? E.g. which parser is this
> > about at all? And how can we actually come in this situation at all?
> > (Whenever I looked at ff_combine_frame() I do not really understand what
> > its invariants are supposed to be.)
>
> Yeah, when I looked at it I also felt like it has all kinds of
> assumptions/preconditions without which it will break, but those
> are not documented. Not really reviewable for correctness.
> The change I proposed to fix the same issue was as below. But
> note that it is not based on actual understanding, just that generally
> index, overread_index and buf_size are updated together so I
> thought it suspicious buf_size was not updated on realloc failure.
> --- a/libavcodec/parser.c
> +++ b/libavcodec/parser.c
> @@ -252,6 +252,7 @@ int ff_combine_frame(ParseContext *pc, int next,
> AV_INPUT_BUFFER_PADDING_SIZE);
> if (!new_buffer) {
> av_log(NULL, AV_LOG_ERROR, "Failed to reallocate parser buffer to
> %d\n", next + pc->index + AV_INPUT_BUFFER_PADDING_SIZE);
> + *buf_size =
> pc->overread_index =
> pc->index = 0;
> return AVERROR(ENOMEM);It appears neither of the 2 fixes was applied this is not ideal I will apply reimars patch thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No snowflake in an avalanche ever feels responsible. -- Voltaire
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
