[FFmpeg-devel] [PATCH] libavformat/dashdec: Fix buffer overflow in segment URL resolution

[jing yan]

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 at niedermayer.cc> 于2025年4月16日周三 07:33写道：

> Hi
>
> On Fri, Apr 11, 2025 at 03:48:08PM +0800, xiaohuanshu at gmail.com wrote:
> > From: xiaohuanshu <xiaohuanshu at 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 at 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 at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>