* [PATCH 00/11] libavformat/asfdec: Fix variable types and add checks for unsupported values
@ 2021-12-22 14:32 ffmpegagent
2021-12-22 14:32 ` [PATCH 01/11] libavformat/asf: fix handling of byte array length values ffmpegagent
` (10 more replies)
0 siblings, 11 replies; 15+ messages in thread
From: ffmpegagent @ 2021-12-22 14:32 UTC (permalink / raw)
To: ffmpegdev; +Cc: softworkz
Fix variable types and add checks for unsupported values and fix other
issues.
softworkz (11):
libavformat/asf: fix handling of byte array length values
libavformat/asfdec: fix get_value return type and add checks for
libavformat/asfdec: fix type of value_len
libavformat/asfdec: fixing get_tag
libavformat/asfdec: implement parsing of GUID values
libavformat/asfdec: remove unused parameters
libavformat/asfdec: fix macro definition and use
libavformat/asfdec: remove variable redefinition in inner scope
libavformat/asfdec: ensure variables are initialized
libavformat/asfdec: fix parameter type in asf_read_stream_propertie()
libavformat/asfdec: fix variable types and add checks for unsupported
values
libavformat/asf.c | 12 +-
libavformat/asf.h | 2 +-
libavformat/asfdec_f.c | 349 ++++++++++++++++++++++++++---------------
3 files changed, 232 insertions(+), 131 deletions(-)
base-commit: 15cfb4eee316a1d6a0764f4460409f0258fd94cb
Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-11%2Fsoftworkz%2Fmaster-upstream_asf-v1
Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-11/softworkz/master-upstream_asf-v1
Pull-Request: https://github.com/ffstaging/FFmpeg/pull/11
--
gitgitgadget
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 01/11] libavformat/asf: fix handling of byte array length values
2021-12-22 14:32 [PATCH 00/11] libavformat/asfdec: Fix variable types and add checks for unsupported values ffmpegagent
@ 2021-12-22 14:32 ` ffmpegagent
2021-12-22 15:24 ` Soft Works
2021-12-22 14:32 ` [PATCH 02/11] libavformat/asfdec: fix get_value return type and add checks for ffmpegagent
` (9 subsequent siblings)
10 siblings, 1 reply; 15+ messages in thread
From: ffmpegagent @ 2021-12-22 14:32 UTC (permalink / raw)
To: ffmpegdev; +Cc: softworkz, softworkz
From: softworkz <softworkz@hotmail.com>
The spec allows attachment sizes of up to UINT32_MAX while
we can handle only sizes up to INT32_MAX (in downstream
code).
The debug.assert in get_tag didn't really address this,
and truncating the value_len in calling methods cannot
be used because the length value is required in order to
continue parsing. This adds a check with log message in
ff_asf_handle_byte_array to handle those (rare) cases.
Signed-off-by: softworkz <softworkz@hotmail.com>
---
libavformat/asf.c | 12 +++++++++---
libavformat/asf.h | 2 +-
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/libavformat/asf.c b/libavformat/asf.c
index 1ac8b5f078..179b66a2b4 100644
--- a/libavformat/asf.c
+++ b/libavformat/asf.c
@@ -267,12 +267,18 @@ static int get_id3_tag(AVFormatContext *s, int len)
}
int ff_asf_handle_byte_array(AVFormatContext *s, const char *name,
- int val_len)
+ uint32_t val_len)
{
+ if (val_len > INT32_MAX) {
+ av_log(s, AV_LOG_VERBOSE, "Unable to handle byte arrays > INT32_MAX in tag %s.\n", name);
+ return 1;
+ }
+
if (!strcmp(name, "WM/Picture")) // handle cover art
- return asf_read_picture(s, val_len);
+ return asf_read_picture(s, (int)val_len);
else if (!strcmp(name, "ID3")) // handle ID3 tag
- return get_id3_tag(s, val_len);
+ return get_id3_tag(s, (int)val_len);
+ av_log(s, AV_LOG_VERBOSE, "Unsupported byte array in tag %s.\n", name);
return 1;
}
diff --git a/libavformat/asf.h b/libavformat/asf.h
index 01cc4f7a46..4d28560f56 100644
--- a/libavformat/asf.h
+++ b/libavformat/asf.h
@@ -111,7 +111,7 @@ extern const AVMetadataConv ff_asf_metadata_conv[];
* is unsupported by this function and 0 otherwise.
*/
int ff_asf_handle_byte_array(AVFormatContext *s, const char *name,
- int val_len);
+ uint32_t val_len);
#define ASF_PACKET_FLAG_ERROR_CORRECTION_PRESENT 0x80 //1000 0000
--
gitgitgadget
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 02/11] libavformat/asfdec: fix get_value return type and add checks for
2021-12-22 14:32 [PATCH 00/11] libavformat/asfdec: Fix variable types and add checks for unsupported values ffmpegagent
2021-12-22 14:32 ` [PATCH 01/11] libavformat/asf: fix handling of byte array length values ffmpegagent
@ 2021-12-22 14:32 ` ffmpegagent
2021-12-22 14:32 ` [PATCH 03/11] libavformat/asfdec: fix type of value_len ffmpegagent
` (8 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: ffmpegagent @ 2021-12-22 14:32 UTC (permalink / raw)
To: ffmpegdev; +Cc: softworkz, softworkz
From: softworkz <softworkz@hotmail.com>
unsupported values
get_value had a return type of int, which means that reading
QWORDS (case 4) was broken due to truncation of the result from
avio_rl64().
Signed-off-by: softworkz <softworkz@hotmail.com>
---
libavformat/asfdec_f.c | 38 +++++++++++++++++++++++++++++---------
1 file changed, 29 insertions(+), 9 deletions(-)
diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
index a8f36ed286..d31e1d581d 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -202,7 +202,7 @@ static int asf_probe(const AVProbeData *pd)
/* size of type 2 (BOOL) is 32bit for "Extended Content Description Object"
* but 16 bit for "Metadata Object" and "Metadata Library Object" */
-static int get_value(AVIOContext *pb, int type, int type2_size)
+static uint64_t get_value(AVIOContext *pb, int type, int type2_size)
{
switch (type) {
case ASF_BOOL:
@@ -567,10 +567,22 @@ static int asf_read_ext_content_desc(AVFormatContext *s, int64_t size)
/* My sample has that stream set to 0 maybe that mean the container.
* ASF stream count starts at 1. I am using 0 to the container value
* since it's unused. */
- if (!strcmp(name, "AspectRatioX"))
- asf->dar[0].num = get_value(s->pb, value_type, 32);
- else if (!strcmp(name, "AspectRatioY"))
- asf->dar[0].den = get_value(s->pb, value_type, 32);
+ if (!strcmp(name, "AspectRatioX")) {
+ const uint64_t value = get_value(s->pb, value_type, 32);
+ if (value > INT32_MAX) {
+ av_log(s, AV_LOG_DEBUG, "Unsupported AspectRatioX value: %"PRIu64"\n", value);
+ return AVERROR(ENOTSUP);
+ }
+ asf->dar[0].num = (int)value;
+ }
+ else if (!strcmp(name, "AspectRatioY")) {
+ const uint64_t value = get_value(s->pb, value_type, 32);
+ if (value > INT32_MAX) {
+ av_log(s, AV_LOG_DEBUG, "Unsupported AspectRatioY value: %"PRIu64"\n", value);
+ return AVERROR(ENOTSUP);
+ }
+ asf->dar[0].den = (int)value;
+ }
else
get_tag(s, name, value_type, value_len, 32);
}
@@ -630,13 +642,21 @@ static int asf_read_metadata(AVFormatContext *s, int64_t size)
i, stream_num, name_len_utf16, value_type, value_len, name);
if (!strcmp(name, "AspectRatioX")){
- int aspect_x = get_value(s->pb, value_type, 16);
+ const uint64_t aspect_x = get_value(s->pb, value_type, 16);
+ if (aspect_x > INT32_MAX) {
+ av_log(s, AV_LOG_DEBUG, "Unsupported AspectRatioX value: %"PRIu64"\n", aspect_x);
+ return AVERROR(ENOTSUP);
+ }
if(stream_num < 128)
- asf->dar[stream_num].num = aspect_x;
+ asf->dar[stream_num].num = (int)aspect_x;
} else if(!strcmp(name, "AspectRatioY")){
- int aspect_y = get_value(s->pb, value_type, 16);
+ const uint64_t aspect_y = get_value(s->pb, value_type, 16);
+ if (aspect_y > INT32_MAX) {
+ av_log(s, AV_LOG_DEBUG, "Unsupported AspectRatioY value: %"PRIu64"\n", aspect_y);
+ return AVERROR(ENOTSUP);
+ }
if(stream_num < 128)
- asf->dar[stream_num].den = aspect_y;
+ asf->dar[stream_num].den = (int)aspect_y;
} else {
get_tag(s, name, value_type, value_len, 16);
}
--
gitgitgadget
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 03/11] libavformat/asfdec: fix type of value_len
2021-12-22 14:32 [PATCH 00/11] libavformat/asfdec: Fix variable types and add checks for unsupported values ffmpegagent
2021-12-22 14:32 ` [PATCH 01/11] libavformat/asf: fix handling of byte array length values ffmpegagent
2021-12-22 14:32 ` [PATCH 02/11] libavformat/asfdec: fix get_value return type and add checks for ffmpegagent
@ 2021-12-22 14:32 ` ffmpegagent
2021-12-22 14:33 ` [PATCH 04/11] libavformat/asfdec: fixing get_tag ffmpegagent
` (7 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: ffmpegagent @ 2021-12-22 14:32 UTC (permalink / raw)
To: ffmpegdev; +Cc: softworkz, softworkz
From: softworkz <softworkz@hotmail.com>
The value_len is an uint32 not an int32 per spec. That
value must not be truncated, neither by casting to int, nor by any
conditional checks, because at the end of get_tag, this value is
needed to move forward in parsing. When the len value gets
modified, the parsing may break.
Signed-off-by: softworkz <softworkz@hotmail.com>
---
libavformat/asfdec_f.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
index d31e1d581d..29b429fee9 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -218,7 +218,7 @@ static uint64_t get_value(AVIOContext *pb, int type, int type2_size)
}
}
-static void get_tag(AVFormatContext *s, const char *key, int type, int len, int type2_size)
+static void get_tag(AVFormatContext *s, const char *key, int type, uint32_t len, int type2_size)
{
ASFContext *asf = s->priv_data;
char *value = NULL;
@@ -528,7 +528,7 @@ static int asf_read_ext_stream_properties(AVFormatContext *s, int64_t size)
static int asf_read_content_desc(AVFormatContext *s, int64_t size)
{
AVIOContext *pb = s->pb;
- int len1, len2, len3, len4, len5;
+ uint32_t len1, len2, len3, len4, len5;
len1 = avio_rl16(pb);
len2 = avio_rl16(pb);
@@ -614,25 +614,23 @@ static int asf_read_metadata(AVFormatContext *s, int64_t size)
{
AVIOContext *pb = s->pb;
ASFContext *asf = s->priv_data;
- int n, stream_num, name_len_utf16, name_len_utf8, value_len;
+ int n, name_len_utf8;
+ uint16_t stream_num, name_len_utf16, value_type;
+ uint32_t value_len;
int ret, i;
n = avio_rl16(pb);
for (i = 0; i < n; i++) {
uint8_t *name;
- int value_type;
avio_rl16(pb); // lang_list_index
- stream_num = avio_rl16(pb);
- name_len_utf16 = avio_rl16(pb);
- value_type = avio_rl16(pb); /* value_type */
- value_len = avio_rl32(pb);
+ stream_num = (uint16_t)avio_rl16(pb);
+ name_len_utf16 = (uint16_t)avio_rl16(pb);
+ value_type = (uint16_t)avio_rl16(pb); /* value_type */
+ value_len = avio_rl32(pb);
- if (value_len < 0 || value_len > UINT16_MAX)
- return AVERROR_INVALIDDATA;
-
- name_len_utf8 = 2*name_len_utf16 + 1;
- name = av_malloc(name_len_utf8);
+ name_len_utf8 = 2 * name_len_utf16 + 1;
+ name = av_malloc(name_len_utf8);
if (!name)
return AVERROR(ENOMEM);
--
gitgitgadget
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 04/11] libavformat/asfdec: fixing get_tag
2021-12-22 14:32 [PATCH 00/11] libavformat/asfdec: Fix variable types and add checks for unsupported values ffmpegagent
` (2 preceding siblings ...)
2021-12-22 14:32 ` [PATCH 03/11] libavformat/asfdec: fix type of value_len ffmpegagent
@ 2021-12-22 14:33 ` ffmpegagent
2021-12-22 14:33 ` [PATCH 05/11] libavformat/asfdec: implement parsing of GUID values ffmpegagent
` (6 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: ffmpegagent @ 2021-12-22 14:33 UTC (permalink / raw)
To: ffmpegdev; +Cc: softworkz, softworkz
From: softworkz <softworkz@hotmail.com>
These three are closely related and can't be separated easily:
In get_tag, the code was adding 22 bytes (in order to allow
it to hold 64bit numbers as string) to the value len for creating
creating a buffer. This was unnecessarily imposing a
size-constraint on the value_len parameter.
The code in get_tag, was limiting the maximum value_len to
half the size of INT32. This was applied for all value types, even
though it is required only in case of ASF_UNICODE, not for any
other ones (like ASCII).
get_tag was always allocating a buffer regardless of the
datatype, even though this isn't required in case of ASF_BYTE_ARRAY
The check for the return value from ff_asf_handle_byte_array()
being >0 is removed here because the log message is emitted
by the function itself now.
Signed-off-by: softworkz <softworkz@hotmail.com>
---
libavformat/asfdec_f.c | 54 +++++++++++++++++++++++++++++++-----------
1 file changed, 40 insertions(+), 14 deletions(-)
diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
index 29b429fee9..58c424b565 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -221,37 +221,63 @@ static uint64_t get_value(AVIOContext *pb, int type, int type2_size)
static void get_tag(AVFormatContext *s, const char *key, int type, uint32_t len, int type2_size)
{
ASFContext *asf = s->priv_data;
- char *value = NULL;
int64_t off = avio_tell(s->pb);
-#define LEN 22
-
- av_assert0((unsigned)len < (INT_MAX - LEN) / 2);
+ char *value = NULL;
+ uint64_t required_bufferlen;
+ int buffer_len;
if (!asf->export_xmp && !strncmp(key, "xmp", 3))
goto finish;
- value = av_malloc(2 * len + LEN);
+ switch (type) {
+ case ASF_UNICODE:
+ required_bufferlen = (uint64_t)len * 2 + 1;
+ break;
+ case -1: // ASCII
+ required_bufferlen = (uint64_t)len + 1;
+ break;
+ case ASF_BYTE_ARRAY:
+ ff_asf_handle_byte_array(s, key, len);
+ goto finish;
+ case ASF_BOOL:
+ case ASF_DWORD:
+ case ASF_QWORD:
+ case ASF_WORD:
+ required_bufferlen = 22;
+ break;
+ case ASF_GUID:
+ required_bufferlen = 33;
+ break;
+ default:
+ required_bufferlen = len;
+ break;
+ }
+
+ if (required_bufferlen > INT32_MAX) {
+ av_log(s, AV_LOG_VERBOSE, "Unable to handle values > INT32_MAX in tag %s.\n", key);
+ goto finish;
+ }
+
+ buffer_len = (int)required_bufferlen;
+
+ value = av_malloc(buffer_len);
if (!value)
goto finish;
switch (type) {
case ASF_UNICODE:
- avio_get_str16le(s->pb, len, value, 2 * len + 1);
+ avio_get_str16le(s->pb, len, value, buffer_len);
break;
- case -1: // ASCI
- avio_read(s->pb, value, len);
- value[len]=0;
+ case -1: // ASCII
+ avio_read(s->pb, value, buffer_len - 1);
+ value[buffer_len - 1] = 0;
break;
- case ASF_BYTE_ARRAY:
- if (ff_asf_handle_byte_array(s, key, len) > 0)
- av_log(s, AV_LOG_VERBOSE, "Unsupported byte array in tag %s.\n", key);
- goto finish;
case ASF_BOOL:
case ASF_DWORD:
case ASF_QWORD:
case ASF_WORD: {
uint64_t num = get_value(s->pb, type, type2_size);
- snprintf(value, LEN, "%"PRIu64, num);
+ snprintf(value, buffer_len, "%"PRIu64, num);
break;
}
case ASF_GUID:
--
gitgitgadget
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 05/11] libavformat/asfdec: implement parsing of GUID values
2021-12-22 14:32 [PATCH 00/11] libavformat/asfdec: Fix variable types and add checks for unsupported values ffmpegagent
` (3 preceding siblings ...)
2021-12-22 14:33 ` [PATCH 04/11] libavformat/asfdec: fixing get_tag ffmpegagent
@ 2021-12-22 14:33 ` ffmpegagent
2021-12-22 14:33 ` [PATCH 06/11] libavformat/asfdec: remove unused parameters ffmpegagent
` (5 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: ffmpegagent @ 2021-12-22 14:33 UTC (permalink / raw)
To: ffmpegdev; +Cc: softworkz, softworkz
From: softworkz <softworkz@hotmail.com>
Signed-off-by: softworkz <softworkz@hotmail.com>
---
libavformat/asfdec_f.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
index 58c424b565..4c898ab3f2 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -280,9 +280,12 @@ static void get_tag(AVFormatContext *s, const char *key, int type, uint32_t len,
snprintf(value, buffer_len, "%"PRIu64, num);
break;
}
- case ASF_GUID:
- av_log(s, AV_LOG_DEBUG, "Unsupported GUID value in tag %s.\n", key);
- goto finish;
+ case ASF_GUID: {
+ ff_asf_guid g;
+ ff_get_guid(s->pb, &g);
+ snprintf(value, buffer_len, "%x", g[0]);
+ break;
+ }
default:
av_log(s, AV_LOG_DEBUG,
"Unsupported value type %d in tag %s.\n", type, key);
--
gitgitgadget
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 06/11] libavformat/asfdec: remove unused parameters
2021-12-22 14:32 [PATCH 00/11] libavformat/asfdec: Fix variable types and add checks for unsupported values ffmpegagent
` (4 preceding siblings ...)
2021-12-22 14:33 ` [PATCH 05/11] libavformat/asfdec: implement parsing of GUID values ffmpegagent
@ 2021-12-22 14:33 ` ffmpegagent
2021-12-22 14:33 ` [PATCH 07/11] libavformat/asfdec: fix macro definition and use ffmpegagent
` (4 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: ffmpegagent @ 2021-12-22 14:33 UTC (permalink / raw)
To: ffmpegdev; +Cc: softworkz, softworkz
From: softworkz <softworkz@hotmail.com>
Signed-off-by: softworkz <softworkz@hotmail.com>
---
libavformat/asfdec_f.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
index 4c898ab3f2..e87c78cd6c 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -299,7 +299,7 @@ finish:
avio_seek(s->pb, off + len, SEEK_SET);
}
-static int asf_read_file_properties(AVFormatContext *s, int64_t size)
+static int asf_read_file_properties(AVFormatContext *s)
{
ASFContext *asf = s->priv_data;
AVIOContext *pb = s->pb;
@@ -494,7 +494,7 @@ static int asf_read_stream_properties(AVFormatContext *s, int64_t size)
return 0;
}
-static int asf_read_ext_stream_properties(AVFormatContext *s, int64_t size)
+static int asf_read_ext_stream_properties(AVFormatContext *s)
{
ASFContext *asf = s->priv_data;
AVIOContext *pb = s->pb;
@@ -554,7 +554,7 @@ static int asf_read_ext_stream_properties(AVFormatContext *s, int64_t size)
return 0;
}
-static int asf_read_content_desc(AVFormatContext *s, int64_t size)
+static int asf_read_content_desc(AVFormatContext *s)
{
AVIOContext *pb = s->pb;
uint32_t len1, len2, len3, len4, len5;
@@ -573,7 +573,7 @@ static int asf_read_content_desc(AVFormatContext *s, int64_t size)
return 0;
}
-static int asf_read_ext_content_desc(AVFormatContext *s, int64_t size)
+static int asf_read_ext_content_desc(AVFormatContext *s)
{
AVIOContext *pb = s->pb;
ASFContext *asf = s->priv_data;
@@ -619,7 +619,7 @@ static int asf_read_ext_content_desc(AVFormatContext *s, int64_t size)
return 0;
}
-static int asf_read_language_list(AVFormatContext *s, int64_t size)
+static int asf_read_language_list(AVFormatContext *s)
{
AVIOContext *pb = s->pb;
ASFContext *asf = s->priv_data;
@@ -639,7 +639,7 @@ static int asf_read_language_list(AVFormatContext *s, int64_t size)
return 0;
}
-static int asf_read_metadata(AVFormatContext *s, int64_t size)
+static int asf_read_metadata(AVFormatContext *s)
{
AVIOContext *pb = s->pb;
ASFContext *asf = s->priv_data;
@@ -693,7 +693,7 @@ static int asf_read_metadata(AVFormatContext *s, int64_t size)
return 0;
}
-static int asf_read_marker(AVFormatContext *s, int64_t size)
+static int asf_read_marker(AVFormatContext *s)
{
AVIOContext *pb = s->pb;
ASFContext *asf = s->priv_data;
@@ -772,21 +772,21 @@ static int asf_read_header(AVFormatContext *s)
if (gsize < 24)
return AVERROR_INVALIDDATA;
if (!ff_guidcmp(&g, &ff_asf_file_header)) {
- ret = asf_read_file_properties(s, gsize);
+ ret = asf_read_file_properties(s);
} else if (!ff_guidcmp(&g, &ff_asf_stream_header)) {
ret = asf_read_stream_properties(s, gsize);
} else if (!ff_guidcmp(&g, &ff_asf_comment_header)) {
- asf_read_content_desc(s, gsize);
+ asf_read_content_desc(s);
} else if (!ff_guidcmp(&g, &ff_asf_language_guid)) {
- asf_read_language_list(s, gsize);
+ asf_read_language_list(s);
} else if (!ff_guidcmp(&g, &ff_asf_extended_content_header)) {
- asf_read_ext_content_desc(s, gsize);
+ asf_read_ext_content_desc(s);
} else if (!ff_guidcmp(&g, &ff_asf_metadata_header)) {
- asf_read_metadata(s, gsize);
+ asf_read_metadata(s);
} else if (!ff_guidcmp(&g, &ff_asf_metadata_library_header)) {
- asf_read_metadata(s, gsize);
+ asf_read_metadata(s);
} else if (!ff_guidcmp(&g, &ff_asf_ext_stream_header)) {
- asf_read_ext_stream_properties(s, gsize);
+ asf_read_ext_stream_properties(s);
// there could be an optional stream properties object to follow
// if so the next iteration will pick it up
@@ -796,7 +796,7 @@ static int asf_read_header(AVFormatContext *s)
avio_skip(pb, 6);
continue;
} else if (!ff_guidcmp(&g, &ff_asf_marker_header)) {
- asf_read_marker(s, gsize);
+ asf_read_marker(s);
} else if (avio_feof(pb)) {
return AVERROR_EOF;
} else {
--
gitgitgadget
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 07/11] libavformat/asfdec: fix macro definition and use
2021-12-22 14:32 [PATCH 00/11] libavformat/asfdec: Fix variable types and add checks for unsupported values ffmpegagent
` (5 preceding siblings ...)
2021-12-22 14:33 ` [PATCH 06/11] libavformat/asfdec: remove unused parameters ffmpegagent
@ 2021-12-22 14:33 ` ffmpegagent
2021-12-22 14:33 ` [PATCH 08/11] libavformat/asfdec: remove variable redefinition in inner scope ffmpegagent
` (3 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: ffmpegagent @ 2021-12-22 14:33 UTC (permalink / raw)
To: ffmpegdev; +Cc: softworkz, softworkz
From: softworkz <softworkz@hotmail.com>
Signed-off-by: softworkz <softworkz@hotmail.com>
---
libavformat/asfdec_f.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
index e87c78cd6c..a7b5ffe465 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -896,21 +896,21 @@ static int asf_read_header(AVFormatContext *s)
}
#define DO_2BITS(bits, var, defval) \
- switch (bits & 3) { \
+ switch ((bits) & 3) { \
case 3: \
- var = avio_rl32(pb); \
+ (var) = avio_rl32(pb); \
rsize += 4; \
break; \
case 2: \
- var = avio_rl16(pb); \
+ (var) = avio_rl16(pb); \
rsize += 2; \
break; \
case 1: \
- var = avio_r8(pb); \
+ (var) = avio_r8(pb); \
rsize++; \
break; \
default: \
- var = defval; \
+ (var) = (defval); \
break; \
}
@@ -993,9 +993,9 @@ static int asf_get_packet(AVFormatContext *s, AVIOContext *pb)
asf->packet_flags = c;
asf->packet_property = d;
- DO_2BITS(asf->packet_flags >> 5, packet_length, s->packet_size);
- DO_2BITS(asf->packet_flags >> 1, padsize, 0); // sequence ignored
- DO_2BITS(asf->packet_flags >> 3, padsize, 0); // padding length
+ DO_2BITS(asf->packet_flags >> 5, packet_length, s->packet_size)
+ DO_2BITS(asf->packet_flags >> 1, padsize, 0) // sequence ignored
+ DO_2BITS(asf->packet_flags >> 3, padsize, 0) // padding length
// the following checks prevent overflows and infinite loops
if (!packet_length || packet_length >= (1U << 29)) {
@@ -1056,9 +1056,9 @@ static int asf_read_frame_header(AVFormatContext *s, AVIOContext *pb)
asf->stream_index = asf->asfid2avid[num & 0x7f];
asfst = &asf->streams[num & 0x7f];
// sequence should be ignored!
- DO_2BITS(asf->packet_property >> 4, asf->packet_seq, 0);
- DO_2BITS(asf->packet_property >> 2, asf->packet_frag_offset, 0);
- DO_2BITS(asf->packet_property, asf->packet_replic_size, 0);
+ DO_2BITS(asf->packet_property >> 4, asf->packet_seq, 0)
+ DO_2BITS(asf->packet_property >> 2, asf->packet_frag_offset, 0)
+ DO_2BITS(asf->packet_property, asf->packet_replic_size, 0)
av_log(asf, AV_LOG_TRACE, "key:%d stream:%d seq:%d offset:%d replic_size:%d num:%X packet_property %X\n",
asf->packet_key_frame, asf->stream_index, asf->packet_seq,
asf->packet_frag_offset, asf->packet_replic_size, num, asf->packet_property);
@@ -1134,7 +1134,7 @@ static int asf_read_frame_header(AVFormatContext *s, AVIOContext *pb)
return AVERROR_INVALIDDATA;
}
if (asf->packet_flags & 0x01) {
- DO_2BITS(asf->packet_segsizetype >> 6, asf->packet_frag_size, 0); // 0 is illegal
+ DO_2BITS(asf->packet_segsizetype >> 6, asf->packet_frag_size, 0) // 0 is illegal
if (rsize > asf->packet_size_left) {
av_log(s, AV_LOG_ERROR, "packet_replic_size is invalid\n");
return AVERROR_INVALIDDATA;
--
gitgitgadget
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 08/11] libavformat/asfdec: remove variable redefinition in inner scope
2021-12-22 14:32 [PATCH 00/11] libavformat/asfdec: Fix variable types and add checks for unsupported values ffmpegagent
` (6 preceding siblings ...)
2021-12-22 14:33 ` [PATCH 07/11] libavformat/asfdec: fix macro definition and use ffmpegagent
@ 2021-12-22 14:33 ` ffmpegagent
2021-12-22 14:33 ` [PATCH 09/11] libavformat/asfdec: ensure variables are initialized ffmpegagent
` (2 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: ffmpegagent @ 2021-12-22 14:33 UTC (permalink / raw)
To: ffmpegdev; +Cc: softworkz, softworkz
From: softworkz <softworkz@hotmail.com>
Signed-off-by: softworkz <softworkz@hotmail.com>
---
libavformat/asfdec_f.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
index a7b5ffe465..8283f245ab 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -1181,7 +1181,7 @@ static int asf_parse_packet(AVFormatContext *s, AVIOContext *pb, AVPacket *pkt)
return AVERROR_EOF;
if (asf->packet_size_left < FRAME_HEADER_SIZE ||
asf->packet_segments < 1 && asf->packet_time_start == 0) {
- int ret = asf->packet_size_left + asf->packet_padsize;
+ ret = asf->packet_size_left + asf->packet_padsize;
if (asf->packet_size_left && asf->packet_size_left < FRAME_HEADER_SIZE)
av_log(s, AV_LOG_WARNING, "Skip due to FRAME_HEADER_SIZE\n");
@@ -1250,7 +1250,6 @@ static int asf_parse_packet(AVFormatContext *s, AVIOContext *pb, AVPacket *pkt)
if (asf_st->pkt.size != asf_st->packet_obj_size ||
// FIXME is this condition sufficient?
asf_st->frag_offset + asf->packet_frag_size > asf_st->pkt.size) {
- int ret;
if (asf_st->pkt.data) {
av_log(s, AV_LOG_INFO,
--
gitgitgadget
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 09/11] libavformat/asfdec: ensure variables are initialized
2021-12-22 14:32 [PATCH 00/11] libavformat/asfdec: Fix variable types and add checks for unsupported values ffmpegagent
` (7 preceding siblings ...)
2021-12-22 14:33 ` [PATCH 08/11] libavformat/asfdec: remove variable redefinition in inner scope ffmpegagent
@ 2021-12-22 14:33 ` ffmpegagent
2021-12-22 14:33 ` [PATCH 10/11] libavformat/asfdec: fix parameter type in asf_read_stream_propertie() ffmpegagent
2021-12-22 14:33 ` [PATCH 11/11] libavformat/asfdec: fix variable types and add checks for unsupported values ffmpegagent
10 siblings, 0 replies; 15+ messages in thread
From: ffmpegagent @ 2021-12-22 14:33 UTC (permalink / raw)
To: ffmpegdev; +Cc: softworkz, softworkz
From: softworkz <softworkz@hotmail.com>
Signed-off-by: softworkz <softworkz@hotmail.com>
---
libavformat/asfdec_f.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
index 8283f245ab..024d77903b 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -968,6 +968,7 @@ static int asf_get_packet(AVFormatContext *s, AVIOContext *pb)
avio_seek(pb, -1, SEEK_CUR); // FIXME
}
} else {
+ d = e = 0;
c = avio_r8(pb);
if (c & 0x80) {
rsize ++;
--
gitgitgadget
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 10/11] libavformat/asfdec: fix parameter type in asf_read_stream_propertie()
2021-12-22 14:32 [PATCH 00/11] libavformat/asfdec: Fix variable types and add checks for unsupported values ffmpegagent
` (8 preceding siblings ...)
2021-12-22 14:33 ` [PATCH 09/11] libavformat/asfdec: ensure variables are initialized ffmpegagent
@ 2021-12-22 14:33 ` ffmpegagent
2021-12-22 14:33 ` [PATCH 11/11] libavformat/asfdec: fix variable types and add checks for unsupported values ffmpegagent
10 siblings, 0 replies; 15+ messages in thread
From: ffmpegagent @ 2021-12-22 14:33 UTC (permalink / raw)
To: ffmpegdev; +Cc: softworkz, softworkz
From: softworkz <softworkz@hotmail.com>
Signed-off-by: softworkz <softworkz@hotmail.com>
---
libavformat/asfdec_f.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
index 024d77903b..b8140a6d57 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -323,7 +323,7 @@ static int asf_read_file_properties(AVFormatContext *s)
return 0;
}
-static int asf_read_stream_properties(AVFormatContext *s, int64_t size)
+static int asf_read_stream_properties(AVFormatContext *s, uint64_t size)
{
ASFContext *asf = s->priv_data;
AVIOContext *pb = s->pb;
--
gitgitgadget
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 11/11] libavformat/asfdec: fix variable types and add checks for unsupported values
2021-12-22 14:32 [PATCH 00/11] libavformat/asfdec: Fix variable types and add checks for unsupported values ffmpegagent
` (9 preceding siblings ...)
2021-12-22 14:33 ` [PATCH 10/11] libavformat/asfdec: fix parameter type in asf_read_stream_propertie() ffmpegagent
@ 2021-12-22 14:33 ` ffmpegagent
10 siblings, 0 replies; 15+ messages in thread
From: ffmpegagent @ 2021-12-22 14:33 UTC (permalink / raw)
To: ffmpegdev; +Cc: softworkz, softworkz
From: softworkz <softworkz@hotmail.com>
Signed-off-by: softworkz <softworkz@hotmail.com>
---
libavformat/asfdec_f.c | 168 ++++++++++++++++++++++++++---------------
1 file changed, 108 insertions(+), 60 deletions(-)
diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
index b8140a6d57..c7141f6da1 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -332,9 +332,9 @@ static int asf_read_stream_properties(AVFormatContext *s, uint64_t size)
ASFStream *asf_st;
ff_asf_guid g;
enum AVMediaType type;
- int type_specific_size, sizeX;
- unsigned int tag1;
- int64_t pos1, pos2, start_time;
+ unsigned int tag1, type_specific_size, sizeX;
+ int64_t pos1, pos2;
+ uint32_t start_time;
int test_for_ext_stream_audio, is_dvr_ms_audio = 0;
if (s->nb_streams == ASF_MAX_STREAMS) {
@@ -403,7 +403,14 @@ static int asf_read_stream_properties(AVFormatContext *s, uint64_t size)
st->codecpar->codec_type = type;
if (type == AVMEDIA_TYPE_AUDIO) {
- int ret = ff_get_wav_header(s, pb, st->codecpar, type_specific_size, 0);
+ int ret;
+
+ if (type_specific_size > INT32_MAX) {
+ av_log(s, AV_LOG_DEBUG, "Unsupported WAV header size (> INT32_MAX)\n");
+ return AVERROR(ENOTSUP);
+ }
+
+ ret = ff_get_wav_header(s, pb, st->codecpar, (int)type_specific_size, 0);
if (ret < 0)
return ret;
if (is_dvr_ms_audio) {
@@ -433,21 +440,32 @@ static int asf_read_stream_properties(AVFormatContext *s, uint64_t size)
}
} else if (type == AVMEDIA_TYPE_VIDEO &&
size - (avio_tell(pb) - pos1 + 24) >= 51) {
+ unsigned int width, height;
avio_rl32(pb);
avio_rl32(pb);
avio_r8(pb);
avio_rl16(pb); /* size */
- sizeX = avio_rl32(pb); /* size */
- st->codecpar->width = avio_rl32(pb);
- st->codecpar->height = avio_rl32(pb);
+ sizeX = avio_rl32(pb); /* size */
+ width = avio_rl32(pb);
+ height = avio_rl32(pb);
+
+ if (width > INT32_MAX || height > INT32_MAX) {
+ av_log(s, AV_LOG_DEBUG, "Unsupported video size %dx%d\n", width, height);
+ return AVERROR(ENOTSUP);
+ }
+
+ st->codecpar->width = (int)width;
+ st->codecpar->height = (int)height;
/* not available for asf */
avio_rl16(pb); /* panes */
st->codecpar->bits_per_coded_sample = avio_rl16(pb); /* depth */
tag1 = avio_rl32(pb);
avio_skip(pb, 20);
if (sizeX > 40) {
- if (size < sizeX - 40 || sizeX - 40 > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE)
- return AVERROR_INVALIDDATA;
+ if (size < sizeX - 40 || sizeX - 40 > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE) {
+ av_log(s, AV_LOG_DEBUG, "Unsupported extradata size\n");
+ return AVERROR(ENOTSUP);
+ }
st->codecpar->extradata_size = ffio_limit(pb, sizeX - 40);
st->codecpar->extradata = av_mallocz(st->codecpar->extradata_size +
AV_INPUT_BUFFER_PADDING_SIZE);
@@ -499,9 +517,9 @@ static int asf_read_ext_stream_properties(AVFormatContext *s)
ASFContext *asf = s->priv_data;
AVIOContext *pb = s->pb;
ff_asf_guid g;
- int ext_len, payload_ext_ct, stream_ct, i;
- uint32_t leak_rate, stream_num;
- unsigned int stream_languageid_index;
+ uint16_t payload_ext_ct, stream_ct, i;
+ uint32_t leak_rate, ext_len;
+ uint16_t stream_languageid_index, stream_num;
avio_rl64(pb); // starttime
avio_rl64(pb); // endtime
@@ -513,15 +531,15 @@ static int asf_read_ext_stream_properties(AVFormatContext *s)
avio_rl32(pb); // alt-init-bucket-fullness
avio_rl32(pb); // max-object-size
avio_rl32(pb); // flags (reliable,seekable,no_cleanpoints?,resend-live-cleanpoints, rest of bits reserved)
- stream_num = avio_rl16(pb); // stream-num
+ stream_num = (uint16_t)avio_rl16(pb); // stream-num
- stream_languageid_index = avio_rl16(pb); // stream-language-id-index
+ stream_languageid_index = (uint16_t)avio_rl16(pb); // stream-language-id-index
if (stream_num < 128)
asf->streams[stream_num].stream_language_index = stream_languageid_index;
avio_rl64(pb); // avg frametime in 100ns units
- stream_ct = avio_rl16(pb); // stream-name-count
- payload_ext_ct = avio_rl16(pb); // payload-extension-system-count
+ stream_ct = (uint16_t)avio_rl16(pb); // stream-name-count
+ payload_ext_ct = (uint16_t)avio_rl16(pb); // payload-extension-system-count
if (stream_num < 128) {
asf->stream_bitrates[stream_num] = leak_rate;
@@ -535,12 +553,10 @@ static int asf_read_ext_stream_properties(AVFormatContext *s)
}
for (i = 0; i < payload_ext_ct; i++) {
- int size;
+ uint16_t size;
ff_get_guid(pb, &g);
- size = avio_rl16(pb);
+ size = (uint16_t)avio_rl16(pb);
ext_len = avio_rl32(pb);
- if (ext_len < 0)
- return AVERROR_INVALIDDATA;
avio_skip(pb, ext_len);
if (stream_num < 128 && i < FF_ARRAY_ELEMS(asf->streams[stream_num].payload)) {
ASFPayload *p = &asf->streams[stream_num].payload[i];
@@ -577,20 +593,21 @@ static int asf_read_ext_content_desc(AVFormatContext *s)
{
AVIOContext *pb = s->pb;
ASFContext *asf = s->priv_data;
- int desc_count, i, ret;
+ uint16_t desc_count, i;
+ int ret;
- desc_count = avio_rl16(pb);
+ desc_count = (uint16_t)avio_rl16(pb);
for (i = 0; i < desc_count; i++) {
- int name_len, value_type, value_len;
+ uint16_t name_len, value_type, value_len;
char name[1024];
- name_len = avio_rl16(pb);
+ name_len = (uint16_t)avio_rl16(pb);
if (name_len % 2) // must be even, broken lavf versions wrote len-1
name_len += 1;
if ((ret = avio_get_str16le(pb, name_len, name, sizeof(name))) < name_len)
avio_skip(pb, name_len - ret);
- value_type = avio_rl16(pb);
- value_len = avio_rl16(pb);
+ value_type = (uint16_t)avio_rl16(pb);
+ value_len = (uint16_t)avio_rl16(pb);
if (!value_type && value_len % 2)
value_len += 1;
/* My sample has that stream set to 0 maybe that mean the container.
@@ -623,14 +640,16 @@ static int asf_read_language_list(AVFormatContext *s)
{
AVIOContext *pb = s->pb;
ASFContext *asf = s->priv_data;
- int j, ret;
- int stream_count = avio_rl16(pb);
+ int ret;
+ uint16_t j;
+ const uint16_t stream_count = (uint16_t)avio_rl16(pb);
+
for (j = 0; j < stream_count; j++) {
char lang[6];
- unsigned int lang_len = avio_r8(pb);
+ const uint8_t lang_len = (uint8_t)avio_r8(pb);
if ((ret = avio_get_str16le(pb, lang_len, lang,
sizeof(lang))) < lang_len)
- avio_skip(pb, lang_len - ret);
+ avio_skip(pb, (int)lang_len - ret);
if (j < 128)
av_strlcpy(asf->stream_languages[j], lang,
sizeof(*asf->stream_languages));
@@ -643,14 +662,14 @@ static int asf_read_metadata(AVFormatContext *s)
{
AVIOContext *pb = s->pb;
ASFContext *asf = s->priv_data;
- int n, name_len_utf8;
- uint16_t stream_num, name_len_utf16, value_type;
+ int name_len_utf8;
+ uint16_t stream_num, name_len_utf16, value_type, i, n;
uint32_t value_len;
- int ret, i;
- n = avio_rl16(pb);
+ int ret;
+ n = (uint16_t)avio_rl16(pb);
for (i = 0; i < n; i++) {
- uint8_t *name;
+ char *name;
avio_rl16(pb); // lang_list_index
stream_num = (uint16_t)avio_rl16(pb);
@@ -664,7 +683,7 @@ static int asf_read_metadata(AVFormatContext *s)
return AVERROR(ENOMEM);
if ((ret = avio_get_str16le(pb, name_len_utf16, name, name_len_utf8)) < name_len_utf16)
- avio_skip(pb, name_len_utf16 - ret);
+ avio_skip(pb, (int)name_len_utf16 - ret);
av_log(s, AV_LOG_TRACE, "%d stream %d name_len %2d type %d len %4d <%s>\n",
i, stream_num, name_len_utf16, value_type, value_len, name);
@@ -697,19 +716,21 @@ static int asf_read_marker(AVFormatContext *s)
{
AVIOContext *pb = s->pb;
ASFContext *asf = s->priv_data;
- int i, count, name_len, ret;
+ int ret;
+ unsigned count, i;
+ uint16_t name_len;
char name[1024];
avio_rl64(pb); // reserved 16 bytes
avio_rl64(pb); // ...
count = avio_rl32(pb); // markers count
avio_rl16(pb); // reserved 2 bytes
- name_len = avio_rl16(pb); // name length
+ name_len = (uint16_t)avio_rl16(pb); // name length
avio_skip(pb, name_len);
for (i = 0; i < count; i++) {
- int64_t pres_time;
- int name_len;
+ uint64_t pres_time;
+ unsigned name2_len;
if (avio_feof(pb))
return AVERROR_INVALIDDATA;
@@ -720,13 +741,18 @@ static int asf_read_marker(AVFormatContext *s)
avio_rl16(pb); // entry length
avio_rl32(pb); // send time
avio_rl32(pb); // flags
- name_len = avio_rl32(pb); // name length
- if ((unsigned)name_len > INT_MAX / 2)
+ name2_len = avio_rl32(pb); // name length
+ if (name2_len > INT_MAX / 2)
return AVERROR_INVALIDDATA;
- if ((ret = avio_get_str16le(pb, name_len * 2, name,
- sizeof(name))) < name_len)
- avio_skip(pb, name_len - ret);
- avpriv_new_chapter(s, i, (AVRational) { 1, 10000000 }, pres_time,
+ if ((ret = avio_get_str16le(pb, (int)name2_len, name,
+ sizeof(name))) < name2_len)
+ avio_skip(pb, name2_len - ret);
+
+ if (pres_time > INT64_MAX) {
+ av_log(s, AV_LOG_DEBUG, "Unsupported presentation time value: %"PRIu64"\n", pres_time);
+ return AVERROR(ENOTSUP);
+ }
+ avpriv_new_chapter(s, i, (AVRational) { 1, 10000000 }, (int64_t)pres_time,
AV_NOPTS_VALUE, name);
}
@@ -739,7 +765,7 @@ static int asf_read_header(AVFormatContext *s)
ff_asf_guid g;
AVIOContext *pb = s->pb;
int i;
- int64_t gsize;
+ uint64_t gsize;
ff_get_guid(pb, &g);
if (ff_guidcmp(&g, &ff_asf_header))
@@ -754,7 +780,7 @@ static int asf_read_header(AVFormatContext *s)
asf->streams[i].stream_language_index = 128; // invalid stream index means no language info
for (;;) {
- uint64_t gpos = avio_tell(pb);
+ const int64_t gpos = avio_tell(pb);
int ret = 0;
ff_get_guid(pb, &g);
gsize = avio_rl64(pb);
@@ -809,7 +835,12 @@ static int asf_read_header(AVFormatContext *s)
len= avio_rl32(pb);
av_log(s, AV_LOG_DEBUG, "Secret data:\n");
- if ((ret = av_get_packet(pb, pkt, len)) < 0)
+ if (len > INT32_MAX) {
+ av_log(s, AV_LOG_DEBUG, "Unsupported encryption packet length: %d\n", len);
+ return AVERROR(ENOTSUP);
+ }
+
+ if ((ret = av_get_packet(pb, pkt, (int)len)) < 0)
return ret;
av_hex_dump_log(s, AV_LOG_DEBUG, pkt->data, pkt->size);
av_packet_unref(pkt);
@@ -923,7 +954,7 @@ static int asf_read_header(AVFormatContext *s)
static int asf_get_packet(AVFormatContext *s, AVIOContext *pb)
{
ASFContext *asf = s->priv_data;
- uint32_t packet_length, padsize;
+ uint32_t packet_length, packet_ts, padsize;
int rsize = 8;
int c, d, e, off;
@@ -1011,7 +1042,12 @@ static int asf_get_packet(AVFormatContext *s, AVIOContext *pb)
return AVERROR_INVALIDDATA;
}
- asf->packet_timestamp = avio_rl32(pb);
+ packet_ts = avio_rl32(pb);
+ if (packet_ts > INT32_MAX) {
+ av_log(s, AV_LOG_DEBUG, "Unsupported packet_timestamp value: %d\n", packet_ts);
+ return AVERROR(ENOTSUP);
+ }
+ asf->packet_timestamp = (int)packet_ts;
avio_rl16(pb); /* duration */
// rsize has at least 11 bytes which have to be present
@@ -1030,10 +1066,21 @@ static int asf_get_packet(AVFormatContext *s, AVIOContext *pb)
rsize, packet_length, padsize, avio_tell(pb));
return AVERROR_INVALIDDATA;
}
- asf->packet_size_left = packet_length - padsize - rsize;
+
+ if (packet_length - padsize - rsize > INT32_MAX) {
+ av_log(s, AV_LOG_DEBUG, "Unsupported packet_size_left value: %d\n", packet_length - padsize - rsize);
+ return AVERROR(ENOTSUP);
+ }
+ asf->packet_size_left = (int)(packet_length - padsize - rsize);
+
if (packet_length < asf->hdr.min_pktsize)
padsize += asf->hdr.min_pktsize - packet_length;
- asf->packet_padsize = padsize;
+ if (padsize > INT32_MAX) {
+ av_log(s, AV_LOG_DEBUG, "Unsupported packet padsize value: %d\n", padsize);
+ return AVERROR(ENOTSUP);
+ }
+
+ asf->packet_padsize = (int)padsize;
av_log(s, AV_LOG_TRACE, "packet: size=%d padsize=%d left=%d\n",
s->packet_size, asf->packet_padsize, asf->packet_size_left);
return 0;
@@ -1068,22 +1115,23 @@ static int asf_read_frame_header(AVFormatContext *s, AVIOContext *pb)
return AVERROR_INVALIDDATA;
}
if (asf->packet_replic_size >= 8) {
- int64_t end = avio_tell(pb) + asf->packet_replic_size;
+ const int64_t end = avio_tell(pb) + asf->packet_replic_size;
AVRational aspect;
- asfst->packet_obj_size = avio_rl32(pb);
- if (asfst->packet_obj_size >= (1 << 24) || asfst->packet_obj_size < 0) {
+ const unsigned packet_obj_size = avio_rl32(pb);
+ if (packet_obj_size >= (1 << 24)) {
av_log(s, AV_LOG_ERROR, "packet_obj_size %d invalid\n", asfst->packet_obj_size);
asfst->packet_obj_size = 0;
return AVERROR_INVALIDDATA;
}
+ asfst->packet_obj_size = (int)packet_obj_size;
asf->packet_frag_timestamp = avio_rl32(pb); // timestamp
for (i = 0; i < asfst->payload_ext_ct; i++) {
ASFPayload *p = &asfst->payload[i];
- int size = p->size;
+ uint16_t size = p->size;
int64_t payend;
if (size == 0xFFFF)
- size = avio_rl16(pb);
+ size = (uint16_t)avio_rl16(pb);
payend = avio_tell(pb) + size;
if (payend > end) {
av_log(s, AV_LOG_ERROR, "too long payload\n");
@@ -1484,7 +1532,7 @@ static int64_t asf_read_pts(AVFormatContext *s, int stream_index,
ASFStream *asf_st;
int64_t pts;
int64_t pos = *ppos;
- int i;
+ unsigned i;
int64_t start_pos[ASF_MAX_STREAMS];
for (i = 0; i < s->nb_streams; i++)
@@ -1541,7 +1589,7 @@ static int asf_build_simple_index(AVFormatContext *s, int stream_index)
int64_t ret;
if((ret = avio_seek(s->pb, asf->data_object_offset + asf->data_object_size, SEEK_SET)) < 0) {
- return ret;
+ return (int)ret;
}
if ((ret = ff_get_guid(s->pb, &g)) < 0)
--
gitgitgadget
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH 01/11] libavformat/asf: fix handling of byte array length values
2021-12-22 14:32 ` [PATCH 01/11] libavformat/asf: fix handling of byte array length values ffmpegagent
@ 2021-12-22 15:24 ` Soft Works
0 siblings, 0 replies; 15+ messages in thread
From: Soft Works @ 2021-12-22 15:24 UTC (permalink / raw)
To: ffmpegdev
> -----Original Message-----
> From: ffmpegagent <ffmpegagent@gmail.com>
> Sent: Wednesday, December 22, 2021 3:33 PM
> To: ffmpegdev@gitmailbox.com
> Cc: softworkz <softworkz@hotmail.com>; softworkz <softworkz@hotmail.com>
> Subject: [PATCH 01/11] libavformat/asf: fix handling of byte array length
> values
>
> From: softworkz <softworkz@hotmail.com>
>
> The spec allows attachment sizes of up to UINT32_MAX while
> we can handle only sizes up to INT32_MAX (in downstream
> code).
>
> The debug.assert in get_tag didn't really address this,
> and truncating the value_len in calling methods cannot
> be used because the length value is required in order to
> continue parsing. This adds a check with log message in
> ff_asf_handle_byte_array to handle those (rare) cases.
>
> Signed-off-by: softworkz <softworkz@hotmail.com>
> ---
> libavformat/asf.c | 12 +++++++++---
> libavformat/asf.h | 2 +-
> 2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/libavformat/asf.c b/libavformat/asf.c
> index 1ac8b5f078..179b66a2b4 100644
> --- a/libavformat/asf.c
> +++ b/libavformat/asf.c
> @@ -267,12 +267,18 @@ static int get_id3_tag(AVFormatContext *s, int len)
> }
>
> int ff_asf_handle_byte_array(AVFormatContext *s, const char *name,
> - int val_len)
> + uint32_t val_len)
> {
> + if (val_len > INT32_MAX) {
> + av_log(s, AV_LOG_VERBOSE, "Unable to handle byte arrays > INT32_MAX
> in tag %s.\n", name);
> + return 1;
> + }
> +
This is a comment!
Thanks
> if (!strcmp(name, "WM/Picture")) // handle cover art
> - return asf_read_picture(s, val_len);
> + return asf_read_picture(s, (int)val_len);
> else if (!strcmp(name, "ID3")) // handle ID3 tag
> - return get_id3_tag(s, val_len);
> + return get_id3_tag(s, (int)val_len);
>
> + av_log(s, AV_LOG_VERBOSE, "Unsupported byte array in tag %s.\n", name);
> return 1;
> }
> diff --git a/libavformat/asf.h b/libavformat/asf.h
> index 01cc4f7a46..4d28560f56 100644
> --- a/libavformat/asf.h
> +++ b/libavformat/asf.h
> @@ -111,7 +111,7 @@ extern const AVMetadataConv ff_asf_metadata_conv[];
> * is unsupported by this function and 0 otherwise.
> */
> int ff_asf_handle_byte_array(AVFormatContext *s, const char *name,
> - int val_len);
> + uint32_t val_len);
>
>
> #define ASF_PACKET_FLAG_ERROR_CORRECTION_PRESENT 0x80 //1000 0000
> --
> gitgitgadget
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH 07/11] libavformat/asfdec: fix macro definition and use
2021-12-22 15:13 ` [PATCH 07/11] libavformat/asfdec: fix macro definition and use ffmpegagent
@ 2021-12-22 16:23 ` Soft Works
0 siblings, 0 replies; 15+ messages in thread
From: Soft Works @ 2021-12-22 16:23 UTC (permalink / raw)
To: ffmpegdev
> -----Original Message-----
> From: ffmpegagent <ffmpegagent@gmail.com>
> Sent: Wednesday, December 22, 2021 4:14 PM
> To: ffmpegdev@gitmailbox.com
> Cc: softworkz <softworkz@hotmail.com>; softworkz <softworkz@hotmail.com>
> Subject: [PATCH 07/11] libavformat/asfdec: fix macro definition and use
>
> From: softworkz <softworkz@hotmail.com>
>
> Signed-off-by: softworkz <softworkz@hotmail.com>
> ---
> libavformat/asfdec_f.c | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
> index e87c78cd6c..a7b5ffe465 100644
> --- a/libavformat/asfdec_f.c
> +++ b/libavformat/asfdec_f.c
> @@ -896,21 +896,21 @@ static int asf_read_header(AVFormatContext *s)
> }
>
> #define DO_2BITS(bits, var, defval) \
> - switch (bits & 3) { \
> + switch ((bits) & 3) { \
> case 3: \
> - var = avio_rl32(pb); \
> + (var) = avio_rl32(pb); \
> rsize += 4; \
> break; \
> case 2: \
> - var = avio_rl16(pb); \
> + (var) = avio_rl16(pb); \
> rsize += 2; \
> break; \
> case 1: \
> - var = avio_r8(pb); \
> + (var) = avio_r8(pb); \
> rsize++; \
> break; \
> default: \
> - var = defval; \
> + (var) = (defval); \
> break; \
> }
>
> @@ -993,9 +993,9 @@ static int asf_get_packet(AVFormatContext *s, AVIOContext
> *pb)
> asf->packet_flags = c;
> asf->packet_property = d;
>
> - DO_2BITS(asf->packet_flags >> 5, packet_length, s->packet_size);
> - DO_2BITS(asf->packet_flags >> 1, padsize, 0); // sequence ignored
> - DO_2BITS(asf->packet_flags >> 3, padsize, 0); // padding length
> + DO_2BITS(asf->packet_flags >> 5, packet_length, s->packet_size)
> + DO_2BITS(asf->packet_flags >> 1, padsize, 0) // sequence ignored
> + DO_2BITS(asf->packet_flags >> 3, padsize, 0) // padding length
>
> // the following checks prevent overflows and infinite loops
> if (!packet_length || packet_length >= (1U << 29)) {
> @@ -1056,9 +1056,9 @@ static int asf_read_frame_header(AVFormatContext *s,
> AVIOContext *pb)
> asf->stream_index = asf->asfid2avid[num & 0x7f];
> asfst = &asf->streams[num & 0x7f];
> // sequence should be ignored!
> - DO_2BITS(asf->packet_property >> 4, asf->packet_seq, 0);
> - DO_2BITS(asf->packet_property >> 2, asf->packet_frag_offset, 0);
> - DO_2BITS(asf->packet_property, asf->packet_replic_size, 0);
> + DO_2BITS(asf->packet_property >> 4, asf->packet_seq, 0)
> + DO_2BITS(asf->packet_property >> 2, asf->packet_frag_offset, 0)
Here's another comment.
> + DO_2BITS(asf->packet_property, asf->packet_replic_size, 0)
> av_log(asf, AV_LOG_TRACE, "key:%d stream:%d seq:%d offset:%d
> replic_size:%d num:%X packet_property %X\n",
> asf->packet_key_frame, asf->stream_index, asf->packet_seq,
> asf->packet_frag_offset, asf->packet_replic_size, num, asf-
> >packet_property);
> @@ -1134,7 +1134,7 @@ static int asf_read_frame_header(AVFormatContext *s,
> AVIOContext *pb)
> return AVERROR_INVALIDDATA;
> }
> if (asf->packet_flags & 0x01) {
> - DO_2BITS(asf->packet_segsizetype >> 6, asf->packet_frag_size, 0); //
> 0 is illegal
> + DO_2BITS(asf->packet_segsizetype >> 6, asf->packet_frag_size, 0) //
> 0 is illegal
> if (rsize > asf->packet_size_left) {
> av_log(s, AV_LOG_ERROR, "packet_replic_size is invalid\n");
> return AVERROR_INVALIDDATA;
> --
> gitgitgadget
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 07/11] libavformat/asfdec: fix macro definition and use
2021-12-22 15:13 [PATCH 00/11] libavformat/asf: fix handling of byte array length values ffmpegagent
@ 2021-12-22 15:13 ` ffmpegagent
2021-12-22 16:23 ` Soft Works
0 siblings, 1 reply; 15+ messages in thread
From: ffmpegagent @ 2021-12-22 15:13 UTC (permalink / raw)
To: ffmpegdev; +Cc: softworkz, softworkz
From: softworkz <softworkz@hotmail.com>
Signed-off-by: softworkz <softworkz@hotmail.com>
---
libavformat/asfdec_f.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
index e87c78cd6c..a7b5ffe465 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -896,21 +896,21 @@ static int asf_read_header(AVFormatContext *s)
}
#define DO_2BITS(bits, var, defval) \
- switch (bits & 3) { \
+ switch ((bits) & 3) { \
case 3: \
- var = avio_rl32(pb); \
+ (var) = avio_rl32(pb); \
rsize += 4; \
break; \
case 2: \
- var = avio_rl16(pb); \
+ (var) = avio_rl16(pb); \
rsize += 2; \
break; \
case 1: \
- var = avio_r8(pb); \
+ (var) = avio_r8(pb); \
rsize++; \
break; \
default: \
- var = defval; \
+ (var) = (defval); \
break; \
}
@@ -993,9 +993,9 @@ static int asf_get_packet(AVFormatContext *s, AVIOContext *pb)
asf->packet_flags = c;
asf->packet_property = d;
- DO_2BITS(asf->packet_flags >> 5, packet_length, s->packet_size);
- DO_2BITS(asf->packet_flags >> 1, padsize, 0); // sequence ignored
- DO_2BITS(asf->packet_flags >> 3, padsize, 0); // padding length
+ DO_2BITS(asf->packet_flags >> 5, packet_length, s->packet_size)
+ DO_2BITS(asf->packet_flags >> 1, padsize, 0) // sequence ignored
+ DO_2BITS(asf->packet_flags >> 3, padsize, 0) // padding length
// the following checks prevent overflows and infinite loops
if (!packet_length || packet_length >= (1U << 29)) {
@@ -1056,9 +1056,9 @@ static int asf_read_frame_header(AVFormatContext *s, AVIOContext *pb)
asf->stream_index = asf->asfid2avid[num & 0x7f];
asfst = &asf->streams[num & 0x7f];
// sequence should be ignored!
- DO_2BITS(asf->packet_property >> 4, asf->packet_seq, 0);
- DO_2BITS(asf->packet_property >> 2, asf->packet_frag_offset, 0);
- DO_2BITS(asf->packet_property, asf->packet_replic_size, 0);
+ DO_2BITS(asf->packet_property >> 4, asf->packet_seq, 0)
+ DO_2BITS(asf->packet_property >> 2, asf->packet_frag_offset, 0)
+ DO_2BITS(asf->packet_property, asf->packet_replic_size, 0)
av_log(asf, AV_LOG_TRACE, "key:%d stream:%d seq:%d offset:%d replic_size:%d num:%X packet_property %X\n",
asf->packet_key_frame, asf->stream_index, asf->packet_seq,
asf->packet_frag_offset, asf->packet_replic_size, num, asf->packet_property);
@@ -1134,7 +1134,7 @@ static int asf_read_frame_header(AVFormatContext *s, AVIOContext *pb)
return AVERROR_INVALIDDATA;
}
if (asf->packet_flags & 0x01) {
- DO_2BITS(asf->packet_segsizetype >> 6, asf->packet_frag_size, 0); // 0 is illegal
+ DO_2BITS(asf->packet_segsizetype >> 6, asf->packet_frag_size, 0) // 0 is illegal
if (rsize > asf->packet_size_left) {
av_log(s, AV_LOG_ERROR, "packet_replic_size is invalid\n");
return AVERROR_INVALIDDATA;
--
gitgitgadget
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-12-22 16:23 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-22 14:32 [PATCH 00/11] libavformat/asfdec: Fix variable types and add checks for unsupported values ffmpegagent
2021-12-22 14:32 ` [PATCH 01/11] libavformat/asf: fix handling of byte array length values ffmpegagent
2021-12-22 15:24 ` Soft Works
2021-12-22 14:32 ` [PATCH 02/11] libavformat/asfdec: fix get_value return type and add checks for ffmpegagent
2021-12-22 14:32 ` [PATCH 03/11] libavformat/asfdec: fix type of value_len ffmpegagent
2021-12-22 14:33 ` [PATCH 04/11] libavformat/asfdec: fixing get_tag ffmpegagent
2021-12-22 14:33 ` [PATCH 05/11] libavformat/asfdec: implement parsing of GUID values ffmpegagent
2021-12-22 14:33 ` [PATCH 06/11] libavformat/asfdec: remove unused parameters ffmpegagent
2021-12-22 14:33 ` [PATCH 07/11] libavformat/asfdec: fix macro definition and use ffmpegagent
2021-12-22 14:33 ` [PATCH 08/11] libavformat/asfdec: remove variable redefinition in inner scope ffmpegagent
2021-12-22 14:33 ` [PATCH 09/11] libavformat/asfdec: ensure variables are initialized ffmpegagent
2021-12-22 14:33 ` [PATCH 10/11] libavformat/asfdec: fix parameter type in asf_read_stream_propertie() ffmpegagent
2021-12-22 14:33 ` [PATCH 11/11] libavformat/asfdec: fix variable types and add checks for unsupported values ffmpegagent
2021-12-22 15:13 [PATCH 00/11] libavformat/asf: fix handling of byte array length values ffmpegagent
2021-12-22 15:13 ` [PATCH 07/11] libavformat/asfdec: fix macro definition and use ffmpegagent
2021-12-22 16:23 ` Soft Works
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