* Re: [FFmpeg-devel] [PATCH] libavformat/dashdec: Fix buffer overflow in segment URL resolution
2025-04-11 7:48 [FFmpeg-devel] [PATCH] libavformat/dashdec: Fix buffer overflow in segment URL resolution xiaohuanshu
@ 2025-04-15 23:32 ` Michael Niedermayer
2025-04-16 5:47 ` jing yan
2025-04-16 6:08 ` [FFmpeg-devel] [PATCH v2] " xiaohuanshu
2025-04-16 6:56 ` [FFmpeg-devel] [PATCH v3] " xiaohuanshu
2 siblings, 1 reply; 6+ messages in thread
From: Michael Niedermayer @ 2025-04-15 23:32 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 3310 bytes --]
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
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: 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".
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [FFmpeg-devel] [PATCH] libavformat/dashdec: Fix buffer overflow in segment URL resolution
2025-04-15 23:32 ` Michael Niedermayer
@ 2025-04-16 5:47 ` jing yan
0 siblings, 0 replies; 6+ messages in thread
From: jing yan @ 2025-04-16 5:47 UTC (permalink / raw)
To: FFmpeg development discussions and patches
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".
^ permalink raw reply [flat|nested] 6+ messages in thread
* [FFmpeg-devel] [PATCH v2] libavformat/dashdec: Fix buffer overflow in segment URL resolution
2025-04-11 7:48 [FFmpeg-devel] [PATCH] libavformat/dashdec: Fix buffer overflow in segment URL resolution xiaohuanshu
2025-04-15 23:32 ` Michael Niedermayer
@ 2025-04-16 6:08 ` xiaohuanshu
2025-04-16 6:11 ` Andreas Rheinhardt
2025-04-16 6:56 ` [FFmpeg-devel] [PATCH v3] " xiaohuanshu
2 siblings, 1 reply; 6+ messages in thread
From: xiaohuanshu @ 2025-04-16 6:08 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: xiaohuanshu
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.
V2:
1. no need to determine whether initialization_val is null.
2. fix the incorrect variable name.
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..a574c91932 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,
+ aligned(strlen(initialization_val) +
+ (rep_id_val ? strlen(rep_id_val) : 0) +
+ (rep_bandwidth_val ? strlen(rep_bandwidth_val) : 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,11 @@ 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,
+ aligned(strlen(media_val) + (rep_id_val ? strlen(rep_id_val) : 0) +
+ (rep_bandwidth_val ? strlen(rep_bandwidth_val) : 0)));
+ max_url_size = max_url_size ? max_url_size : MAX_URL_SIZE;
seg->url = get_content_url(baseurl_nodes, 4,
max_url_size,
rep_id_val,
@@ -2369,3 +2380,4 @@ const FFInputFormat ff_dash_demuxer = {
.read_close = dash_close,
.read_seek = dash_read_seek,
};
+
--
2.39.5 (Apple Git-154)
_______________________________________________
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".
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [FFmpeg-devel] [PATCH v2] libavformat/dashdec: Fix buffer overflow in segment URL resolution
2025-04-16 6:08 ` [FFmpeg-devel] [PATCH v2] " xiaohuanshu
@ 2025-04-16 6:11 ` Andreas Rheinhardt
0 siblings, 0 replies; 6+ messages in thread
From: Andreas Rheinhardt @ 2025-04-16 6:11 UTC (permalink / raw)
To: ffmpeg-devel
xiaohuanshu@gmail.com:
> 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.
>
> V2:
> 1. no need to determine whether initialization_val is null.
> 2. fix the incorrect variable name.
>
> 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..a574c91932 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;
This should use way smaller scope.
> 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,
> + aligned(strlen(initialization_val) +
> + (rep_id_val ? strlen(rep_id_val) : 0) +
> + (rep_bandwidth_val ? strlen(rep_bandwidth_val) : 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,11 @@ 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,
> + aligned(strlen(media_val) + (rep_id_val ? strlen(rep_id_val) : 0) +
> + (rep_bandwidth_val ? strlen(rep_bandwidth_val) : 0)));
> + max_url_size = max_url_size ? max_url_size : MAX_URL_SIZE;
> seg->url = get_content_url(baseurl_nodes, 4,
> max_url_size,
> rep_id_val,
> @@ -2369,3 +2380,4 @@ const FFInputFormat ff_dash_demuxer = {
> .read_close = dash_close,
> .read_seek = dash_read_seek,
> };
> +
Stray change
_______________________________________________
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".
^ permalink raw reply [flat|nested] 6+ messages in thread
* [FFmpeg-devel] [PATCH v3] libavformat/dashdec: Fix buffer overflow in segment URL resolution
2025-04-11 7:48 [FFmpeg-devel] [PATCH] libavformat/dashdec: Fix buffer overflow in segment URL resolution xiaohuanshu
2025-04-15 23:32 ` Michael Niedermayer
2025-04-16 6:08 ` [FFmpeg-devel] [PATCH v2] " xiaohuanshu
@ 2025-04-16 6:56 ` xiaohuanshu
2 siblings, 0 replies; 6+ messages in thread
From: xiaohuanshu @ 2025-04-16 6:56 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: xiaohuanshu
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.
V2:
1. no need to determine whether initialization_val is null.
2. fix the incorrect variable name.
V3:
1. change `max_url_size` scope into `Initialization` and `Media` blocks.
Signed-off-by: xiaohuanshu <xiaohuanshu@gmail.com>
---
libavformat/dashdec.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index c3f3d7f3f8..31a84bd184 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -606,7 +606,6 @@ 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 err;
if (!av_strcasecmp(fragmenturl_node->name, "Initialization")) {
@@ -620,6 +619,12 @@ static int parse_manifest_segmenturlnode(AVFormatContext *s, struct representati
xmlFree(initialization_val);
return AVERROR(ENOMEM);
}
+ int max_url_size = FFMAX(
+ c ? c->max_url_size : 0,
+ aligned(strlen(initialization_val) +
+ (rep_id_val ? strlen(rep_id_val) : 0) +
+ (rep_bandwidth_val ? strlen(rep_bandwidth_val) : 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 +646,11 @@ static int parse_manifest_segmenturlnode(AVFormatContext *s, struct representati
xmlFree(media_val);
return AVERROR(ENOMEM);
}
+ int max_url_size = FFMAX(
+ c ? c->max_url_size : 0,
+ aligned(strlen(media_val) + (rep_id_val ? strlen(rep_id_val) : 0) +
+ (rep_bandwidth_val ? strlen(rep_bandwidth_val) : 0)));
+ max_url_size = max_url_size ? max_url_size : MAX_URL_SIZE;
seg->url = get_content_url(baseurl_nodes, 4,
max_url_size,
rep_id_val,
--
2.39.5 (Apple Git-154)
_______________________________________________
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".
^ permalink raw reply [flat|nested] 6+ messages in thread