diff mbox series

[FFmpeg-devel,RFC] avformat/hls: check IV size inside EXT-X-KEY

Message ID tencent_9D38D0ACE6D667FB2A84D32B5C5A4BAA2606@qq.com
State New
Headers show
Series [FFmpeg-devel,RFC] avformat/hls: check IV size inside EXT-X-KEY | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_armv7_RPi4 success Make finished
andriy/make_fate_armv7_RPi4 success Make fate finished

Commit Message

zhilizhao(赵志立) April 12, 2022, 8:27 a.m. UTC
IV should always be 128 bits. If the IV attribute was truncated
inside EXT-X-KEY, padding on the left which is the same as when
using sequence number as IV.
---
I'm not sure which method is better: do padding or just return
AVERROR_INVALIDDATA?

 libavformat/hls.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Steven Liu April 12, 2022, 8:32 a.m. UTC | #1
Zhao Zhili <quinkblack@foxmail.com> 于2022年4月12日周二 16:28写道:
>
> IV should always be 128 bits. If the IV attribute was truncated
> inside EXT-X-KEY, padding on the left which is the same as when
> using sequence number as IV.
> ---
> I'm not sure which method is better: do padding or just return
> AVERROR_INVALIDDATA?
>
>  libavformat/hls.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index 83ff4cc607..a7b632c20e 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -810,7 +810,13 @@ static int parse_playlist(HLSContext *c, const char *url,
>              if (!strcmp(info.method, "SAMPLE-AES"))
>                  key_type = KEY_SAMPLE_AES;
>              if (!av_strncasecmp(info.iv, "0x", 2)) {
> -                ff_hex_to_data(iv, info.iv + 2);
> +                int n = ff_hex_to_data(iv, info.iv + 2);
> +                if (n < sizeof(iv)) {
> +                    av_log(c->ctx, AV_LOG_WARNING,
> +                           "Incomplete IV %s, padding on the left\n", info.iv);
> +                    memmove(iv + sizeof(iv) - n, iv, n);
> +                    memset(iv, 0, sizeof(iv) - n);
> +                }
>                  has_iv = 1;
>              }
>              av_strlcpy(key, info.uri, sizeof(key));
> --
> 2.31.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".

LGTM


Thanks
Steven
Anton Khirnov April 13, 2022, 11:53 a.m. UTC | #2
Quoting Zhao Zhili (2022-04-12 10:27:50)
> IV should always be 128 bits. If the IV attribute was truncated
> inside EXT-X-KEY, padding on the left which is the same as when
> using sequence number as IV.
> ---
> I'm not sure which method is better: do padding or just return
> AVERROR_INVALIDDATA?

I would say return error, unless there are known buggy producers of
incomplete IVs that we want to support.
zhilizhao(赵志立) April 13, 2022, 12:32 p.m. UTC | #3
> On Apr 13, 2022, at 7:53 PM, Anton Khirnov <anton@khirnov.net> wrote:
> 
> Quoting Zhao Zhili (2022-04-12 10:27:50)
>> IV should always be 128 bits. If the IV attribute was truncated
>> inside EXT-X-KEY, padding on the left which is the same as when
>> using sequence number as IV.
>> ---
>> I'm not sure which method is better: do padding or just return
>> AVERROR_INVALIDDATA?
> 
> I would say return error, unless there are known buggy producers of
> incomplete IVs that we want to support.

Make sense. Actually it’s a story about shoot myself in the foot. I
created a buggy manifest, and the bug is hidden by the default
padding on the tail. So the vide only works with ffmpeg/ffplay. Let’s
hope I’m the only one make such mistake.

Will send a patch to return error in this case.

> 
> -- 
> Anton Khirnov
> _______________________________________________
> 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".
diff mbox series

Patch

diff --git a/libavformat/hls.c b/libavformat/hls.c
index 83ff4cc607..a7b632c20e 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -810,7 +810,13 @@  static int parse_playlist(HLSContext *c, const char *url,
             if (!strcmp(info.method, "SAMPLE-AES"))
                 key_type = KEY_SAMPLE_AES;
             if (!av_strncasecmp(info.iv, "0x", 2)) {
-                ff_hex_to_data(iv, info.iv + 2);
+                int n = ff_hex_to_data(iv, info.iv + 2);
+                if (n < sizeof(iv)) {
+                    av_log(c->ctx, AV_LOG_WARNING,
+                           "Incomplete IV %s, padding on the left\n", info.iv);
+                    memmove(iv + sizeof(iv) - n, iv, n);
+                    memset(iv, 0, sizeof(iv) - n);
+                }
                 has_iv = 1;
             }
             av_strlcpy(key, info.uri, sizeof(key));