From: jing yan <xiaohuanshu@gmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH] libavformat/dashdec: Fix buffer overflow in segment URL resolution
Date: Wed, 16 Apr 2025 13:47:56 +0800
Message-ID: <CAB2PkDeJRHdx2tViOkutH9xsPag8gkb9OBPpiyNEN2XpMHXCCQ@mail.gmail.com> (raw)
In-Reply-To: <20250415233254.GX4991@pb2>
Thanks for the review!
that can't be null and I found another mistake.
I will fix this issue and send a v2 patch soon.
Michael Niedermayer <michael@niedermayer.cc> 于2025年4月16日周三 07:33写道:
> Hi
>
> On Fri, Apr 11, 2025 at 03:48:08PM +0800, xiaohuanshu@gmail.com wrote:
> > From: xiaohuanshu <xiaohuanshu@gmail.com>
> >
> > Problem:
> > The max_url_size calculation for DASH segment URLs only considered the
> base URL
> > length, leading to buffer overflow when the segment's sourceURL exceeded
> the
> > pre-allocated buffer. This triggered the log error:
> > "DASH request for url 'invalid:truncated'".
> >
> > Reproduce:
> > 1. A test sample "long-sourceurl-sample.mpd" (deliberately designed with
> a long
> > sourceURL) was uploaded to VideoLAN's repository.
> > 2. Reproduce with short base path:
> > ffmpeg -i /tmp/short_path/long-sourceurl-sample.mpd
> > -> Triggers "invalid:truncated" error
> > 3. With artificially lengthened base path (e.g. /aaa/../bbb/../...):
> > ffmpeg -i
> /long/../path/../with/../many/../segments/long-sourceurl-sample.mpd
> > -> URL resolves correctly (though HTTP fetch fails due to fake URL)
> >
> > Fix:
> > Recalculate max_url_size by considering both base URL and sourceURL
> lengths,
> > ensuring sufficient buffer allocation during URL concatenation.
> >
> > Signed-off-by: xiaohuanshu <xiaohuanshu@gmail.com>
> > ---
> > libavformat/dashdec.c | 14 +++++++++++++-
> > 1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
> > index c3f3d7f3f8..f604d363c4 100644
> > --- a/libavformat/dashdec.c
> > +++ b/libavformat/dashdec.c
> > @@ -606,7 +606,7 @@ static int
> parse_manifest_segmenturlnode(AVFormatContext *s, struct representati
> > char *initialization_val = NULL;
> > char *media_val = NULL;
> > char *range_val = NULL;
> > - int max_url_size = c ? c->max_url_size: MAX_URL_SIZE;
> > + int max_url_size = 0;
> > int err;
> >
> > if (!av_strcasecmp(fragmenturl_node->name, "Initialization")) {
> > @@ -620,6 +620,12 @@ static int
> parse_manifest_segmenturlnode(AVFormatContext *s, struct representati
> > xmlFree(initialization_val);
> > return AVERROR(ENOMEM);
> > }
> > + max_url_size = FFMAX(
>
> > + c ? c->max_url_size : 0,
>
> how can c be NULL here ?
>
>
> > + initialization_val ? aligned(strlen(initialization_val)
> +
> > + (rep_id_val ?
> strlen(rep_id_val) : 0) +
> > + (rep_bandwidth_val ?
> strlen(rep_bandwidth_val) : 0)) : 0);
> > + max_url_size = max_url_size ? max_url_size : MAX_URL_SIZE;
> > rep->init_section->url = get_content_url(baseurl_nodes, 4,
> > max_url_size,
> > rep_id_val,
> > @@ -641,6 +647,12 @@ static int
> parse_manifest_segmenturlnode(AVFormatContext *s, struct representati
> > xmlFree(media_val);
> > return AVERROR(ENOMEM);
> > }
> > + max_url_size = FFMAX(
> > + c ? c->max_url_size : 0,
>
> > + initialization_val ? aligned(strlen(initialization_val)
> +
>
> how can initialization_val be non NULL here ?
>
> thx
>
> [...]
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> I have often repented speaking, but never of holding my tongue.
> -- Xenocrates
> _______________________________________________
> 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".
>
_______________________________________________
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".
next prev parent reply other threads:[~2025-04-16 5:48 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-11 7:48 xiaohuanshu
2025-04-15 23:32 ` Michael Niedermayer
2025-04-16 5:47 ` jing yan [this message]
2025-04-16 6:08 ` [FFmpeg-devel] [PATCH v2] " xiaohuanshu
2025-04-16 6:11 ` Andreas Rheinhardt
2025-04-16 6:56 ` [FFmpeg-devel] [PATCH v3] " xiaohuanshu
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=CAB2PkDeJRHdx2tViOkutH9xsPag8gkb9OBPpiyNEN2XpMHXCCQ@mail.gmail.com \
--to=xiaohuanshu@gmail.com \
--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