Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Timo Rothenpieler <timo@rothenpieler.org>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH 1/5] avformat/flvdec: Check for EOF in AudioPacketTypeMultichannelConfig
Date: Mon, 14 Jul 2025 22:00:19 +0200
Message-ID: <71ce56e6-5d83-41f7-9872-e81e0a342cf9@rothenpieler.org> (raw)
In-Reply-To: <20250714192133.GL29660@pb2>

[-- Attachment #1: Type: text/plain, Size: 2451 bytes --]

On 7/14/2025 9:21 PM, Michael Niedermayer wrote:
> On Sun, Jul 13, 2025 at 01:42:28PM +0200, Timo Rothenpieler wrote:
>> On 7/13/2025 3:10 AM, Michael Niedermayer wrote:
>>> Fixes: Infinite loop
>>> Fixes: 427538726/clusterfuzz-testcase-minimized-ffmpeg_dem_FLV_fuzzer-6582567304495104
>>>
>>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>>> ---
>>>    libavformat/flvdec.c | 3 +++
>>>    1 file changed, 3 insertions(+)
>>>
>>> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
>>> index ac681954cb7..a4fa0157512 100644
>>> --- a/libavformat/flvdec.c
>>> +++ b/libavformat/flvdec.c
>>> @@ -1715,6 +1715,9 @@ retry_duration:
>>>                    av_log(s, AV_LOG_DEBUG, "Set channel data from MultiChannel info.\n");
>>> +                if (avio_feof(s->pb))
>>> +                    return AVERROR_EOF;
>>> +
>>>                    goto next_track;
>>>                }
>>>            } else if (stream_type == FLV_STREAM_TYPE_VIDEO) {
>>
>> I don't think just returning from here is correct.
>> The goto next_track right after it already checks for EOF.
> 
>> I do not see how between here and the eof check there there'd be any way to
>> infinite loop.
> 
> avio_skip() with a negative value will reset the EOF flag
> 
> 
>>
>> It returns FFERROR_REDO there, which is important to drain queued up
>> packages.
> 
> I think the state becomes corrupted once it reads into EOF
> that is the size accounting goes "oops" as the code keeps running
> things that read and keeps accounting for these reads but in reality
> nothing is read as its at EOF
> and then it seeks back all these "not reads" and thats where it hits the
> infinite loop as theres a mismatch what the code thinks it moved forward
> and what it actually moved forward.
> Thats how it looked to me at least, i have not verified every step of this
> 
> ill mail you the testcase, then you can check if my analysis is right
> and fix the code in a way that can recover queued packets in such truncated
> packet at EOF case.
> Also please make sure its not forgotten that whatever fix this gets is backported
I'm unable to reproduce any infinite loops, even with the sample.
But the code there definitely is sub-optimal, given the seek can go the 
wrong way, and even when going the right way can potentially reset the 
EOF flag.

Proposed patch is attached.

[-- Attachment #2: 0001-avformat-flvdec-don-t-skip-backwards-or-at-EOF.patch --]
[-- Type: text/plain, Size: 1150 bytes --]

From 7ff394e1ecab504a4cb0fda4bd0f25d88ee4f6fe Mon Sep 17 00:00:00 2001
From: Timo Rothenpieler <timo@rothenpieler.org>
Date: Mon, 14 Jul 2025 21:54:35 +0200
Subject: [PATCH] avformat/flvdec: don't skip backwards or at EOF

---
 libavformat/flvdec.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
index b90ed34b1c..de5e688822 100644
--- a/libavformat/flvdec.c
+++ b/libavformat/flvdec.c
@@ -1860,8 +1860,16 @@ retry_duration:
 next_track:
         if (track_size) {
             av_log(s, AV_LOG_WARNING, "Track size mismatch: %d!\n", track_size);
-            avio_skip(s->pb, track_size);
-            size -= track_size;
+            if (!avio_feof(s->pb)) {
+                if (track_size > 0) {
+                    avio_skip(s->pb, track_size);
+                    size -= track_size;
+                } else {
+                    /* We have somehow read more than the track had to offer, leave and re-sync */
+                    ret = FFERROR_REDO;
+                    goto leave;
+                }
+            }
         }
 
         if (!size)
-- 
2.49.0


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

  parent reply	other threads:[~2025-07-14 20:00 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-13  1:10 Michael Niedermayer
2025-07-13  1:10 ` [FFmpeg-devel] [PATCH 2/5] avformat/concatdec: Clip duration in one more case in get_best_effort_duration() Michael Niedermayer
2025-07-13  1:10 ` [FFmpeg-devel] [PATCH 3/5] avcodec/h264chroma_template: Replace variable by constant in chroma mc Michael Niedermayer
2025-07-14 16:49   ` Kieran Kunhya via ffmpeg-devel
2025-07-13  1:10 ` [FFmpeg-devel] [PATCH 4/5] avcodec/mpegvideo_dec: Fix lowres=3 field select interlaced mpeg4 frame Michael Niedermayer
2025-07-13 17:34   ` Andreas Rheinhardt
2025-07-14 18:52     ` Michael Niedermayer
2025-07-13  1:10 ` [FFmpeg-devel] [PATCH 5/5] avcodec/osq: Fix 32bit sample overflow Michael Niedermayer
2025-07-13 17:37   ` Andreas Rheinhardt
2025-07-14 18:58     ` Michael Niedermayer
2025-07-13 11:42 ` [FFmpeg-devel] [PATCH 1/5] avformat/flvdec: Check for EOF in AudioPacketTypeMultichannelConfig Timo Rothenpieler
2025-07-14 19:21   ` Michael Niedermayer
2025-07-14 19:25     ` Timo Rothenpieler
2025-07-14 20:00     ` Timo Rothenpieler [this message]
2025-07-14 22:01       ` Michael Niedermayer
2025-07-14 22:28         ` Timo Rothenpieler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=71ce56e6-5d83-41f7-9872-e81e0a342cf9@rothenpieler.org \
    --to=timo@rothenpieler.org \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git