On 9/29/2021 11:58 PM, Soft Works wrote:
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 <softwo...@hotmail.com>
---
v5: Split into pieces as requested

  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 076b5ab147..d017fae019 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)

There's an av_assert0() in this function to ensure len will never be bigger than (INT_MAX - 22) / 2. And len is used as argument for avio_read(), which takes an int, not an unsigned int. So this change is not ok.

  {
      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);
@@ -602,25 +602,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);

Unrelated cosmetics.

          if (!name)
              return AVERROR(ENOMEM);

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to