[FFmpeg-devel] [PATCH 1/2] avformat/dashdec: Fix crash on invalid input/ENOMEM, fix leak

[FFmpeg-devel] [PATCH 1/2] avformat/dashdec: Fix crash on invalid input/ENOMEM, fix leak

[Andreas Rheinhardt]

In case a SupplementalProperty node exists in an adaptationset,
it is searched for a "schemeIdUri" property via xmlGetProp().
Whatever xmlGetProp() returns is then compared via av_strcasecmp()
to a string literal. xmlGetProp() can return NULL, namely in case
no "schemeIdUri" exists and (given that this string is allocated)
presumably also on allocation failure. No check for NULL is done,
so this may crash.

Furthermore, the string returned by xmlGetProp() needs to be freed
with xmlFree(), but this is not done either.

This commit fixes both of these issues; they existed since this code
has been added in 10d008f0fd9e713e290f626300d66382ad786c49.

This has been found while investigating ticket #9697. The continuous
leaks might very well be the reason behind the observed slowdown.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavformat/dashdec.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index 63bf7e96a5..2ca91bea8b 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -956,7 +956,11 @@ static int parse_manifest_representation(AVFormatContext *s, const char *url,
             xmlFree(val);
         }
         if (adaptionset_supplementalproperty_node) {
-            if (!av_strcasecmp(xmlGetProp(adaptionset_supplementalproperty_node,"schemeIdUri"), "http://dashif.org/guidelines/last-segment-number")) {
+            char *scheme_id_uri = xmlGetProp(adaptionset_supplementalproperty_node, "schemeIdUri");
+            if (scheme_id_uri) {
+                int is_last_segment_number = !av_strcasecmp(scheme_id_uri, "http://dashif.org/guidelines/last-segment-number");
+                xmlFree(scheme_id_uri);
+                if (is_last_segment_number) {
                 val = xmlGetProp(adaptionset_supplementalproperty_node,"value");
                 if (!val) {
                     av_log(s, AV_LOG_ERROR, "Missing value attribute in adaptionset_supplementalproperty_node\n");
@@ -965,6 +969,7 @@ static int parse_manifest_representation(AVFormatContext *s, const char *url,
                     xmlFree(val);
                 }
             }
+            }
         }
 
         fragment_timeline_node = find_child_node_by_name(representation_segmenttemplate_node, "SegmentTimeline");
-- 
2.34.1

_______________________________________________
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] [PATCH 2/2] avformat/dashdec: Reindent after the previous commit

[Andreas Rheinhardt]

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavformat/dashdec.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index 2ca91bea8b..29d4680c68 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -961,15 +961,15 @@ static int parse_manifest_representation(AVFormatContext *s, const char *url,
                 int is_last_segment_number = !av_strcasecmp(scheme_id_uri, "http://dashif.org/guidelines/last-segment-number");
                 xmlFree(scheme_id_uri);
                 if (is_last_segment_number) {
-                val = xmlGetProp(adaptionset_supplementalproperty_node,"value");
-                if (!val) {
-                    av_log(s, AV_LOG_ERROR, "Missing value attribute in adaptionset_supplementalproperty_node\n");
-                } else {
-                    rep->last_seq_no =(int64_t) strtoll(val, NULL, 10) - 1;
-                    xmlFree(val);
+                    val = xmlGetProp(adaptionset_supplementalproperty_node, "value");
+                    if (!val) {
+                        av_log(s, AV_LOG_ERROR, "Missing value attribute in adaptionset_supplementalproperty_node\n");
+                    } else {
+                        rep->last_seq_no = (int64_t)strtoll(val, NULL, 10) - 1;
+                        xmlFree(val);
+                    }
                 }
             }
-            }
         }
 
         fragment_timeline_node = find_child_node_by_name(representation_segmenttemplate_node, "SegmentTimeline");
-- 
2.34.1

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/2] avformat/dashdec: Fix crash on invalid input/ENOMEM, fix leak

[Steven Liu]

Andreas Rheinhardt <andreas.rheinhardt@outlook.com> 于2022年9月21日周三 04:25写道：
>
> In case a SupplementalProperty node exists in an adaptationset,
> it is searched for a "schemeIdUri" property via xmlGetProp().
> Whatever xmlGetProp() returns is then compared via av_strcasecmp()
> to a string literal. xmlGetProp() can return NULL, namely in case
> no "schemeIdUri" exists and (given that this string is allocated)
> presumably also on allocation failure. No check for NULL is done,
> so this may crash.
>
> Furthermore, the string returned by xmlGetProp() needs to be freed
> with xmlFree(), but this is not done either.
>
> This commit fixes both of these issues; they existed since this code
> has been added in 10d008f0fd9e713e290f626300d66382ad786c49.
>
> This has been found while investigating ticket #9697. The continuous
> leaks might very well be the reason behind the observed slowdown.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavformat/dashdec.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
> index 63bf7e96a5..2ca91bea8b 100644
> --- a/libavformat/dashdec.c
> +++ b/libavformat/dashdec.c
> @@ -956,7 +956,11 @@ static int parse_manifest_representation(AVFormatContext *s, const char *url,
>              xmlFree(val);
>          }
>          if (adaptionset_supplementalproperty_node) {
> -            if (!av_strcasecmp(xmlGetProp(adaptionset_supplementalproperty_node,"schemeIdUri"), "http://dashif.org/guidelines/last-segment-number")) {
> +            char *scheme_id_uri = xmlGetProp(adaptionset_supplementalproperty_node, "schemeIdUri");
> +            if (scheme_id_uri) {
> +                int is_last_segment_number = !av_strcasecmp(scheme_id_uri, "http://dashif.org/guidelines/last-segment-number");
> +                xmlFree(scheme_id_uri);
> +                if (is_last_segment_number) {
>                  val = xmlGetProp(adaptionset_supplementalproperty_node,"value");
>                  if (!val) {
>                      av_log(s, AV_LOG_ERROR, "Missing value attribute in adaptionset_supplementalproperty_node\n");
> @@ -965,6 +969,7 @@ static int parse_manifest_representation(AVFormatContext *s, const char *url,
>                      xmlFree(val);
>                  }
>              }
> +            }
>          }
>
>          fragment_timeline_node = find_child_node_by_name(representation_segmenttemplate_node, "SegmentTimeline");
> --
> 2.34.1
>
> _______________________________________________
> 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".


patchset lgtm


Thanks
Steven
_______________________________________________
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".