[FFmpeg-devel] [PATCH 1/5] avformat/wtvdec: Don't truncate GUIDs

[FFmpeg-devel] [PATCH 1/5] avformat/wtvdec: Don't truncate GUIDs

[Andreas Rheinhardt]

Each of the 16 bytes of a GUID is written as a two-character
hex value and three hyphens, leading to a length of 35.
GCC 13 emits a -Wformat-truncation= warning because of this.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
Is there actually a reason this is using a different format than
the one used by lavu/uuid.h?

 libavformat/wtvdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/wtvdec.c b/libavformat/wtvdec.c
index b29ea7a923..1103f5ba03 100644
--- a/libavformat/wtvdec.c
+++ b/libavformat/wtvdec.c
@@ -43,7 +43,7 @@
     "%08"PRIx32"-%04"PRIx16"-%04"PRIx16"-%02x%02x%02x%02x%02x%02x%02x%02x"
 #define ARG_PRETTY_GUID(g) \
     AV_RL32(g),AV_RL16(g+4),AV_RL16(g+6),g[8],g[9],g[10],g[11],g[12],g[13],g[14],g[15]
-#define LEN_PRETTY_GUID 34
+#define LEN_PRETTY_GUID 35
 
 /*
  * File system routines
-- 
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/5] avformat/wtvdec: Skip too big tags

[Andreas Rheinhardt]

get_tag() is not designed with negative length in mind;
in this case, it will allocate a very small buffer
(LEN_PRETTY_GUID + 1) and might call avio_get_str16le()
with a negative maxlen (which relies on these parameters
to be signed).

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavformat/wtvdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/wtvdec.c b/libavformat/wtvdec.c
index 1103f5ba03..2de6dc2103 100644
--- a/libavformat/wtvdec.c
+++ b/libavformat/wtvdec.c
@@ -539,7 +539,7 @@ static void parse_legacy_attrib(AVFormatContext *s, AVIOContext *pb)
         ff_get_guid(pb, &guid);
         type   = avio_rl32(pb);
         length = avio_rl32(pb);
-        if (!length)
+        if (length <= 0)
             break;
         if (ff_guidcmp(&guid, ff_metadata_guid)) {
             av_log(s, AV_LOG_WARNING, "unknown guid "FF_PRI_GUID", expected metadata_guid; "
-- 
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 3/5] avformat/wtvdec: Fix signed integer overflow

[Andreas Rheinhardt]

Happens when length > INT_MAX / 2; use unsigned for the computation,
but restrict the value to INT_MAX, because avio_get_str16le()
accepts an int as buf_len argument. Notice that it can happen
that the string read by avio_get_str16le() is truncated in this case.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavformat/wtvdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/wtvdec.c b/libavformat/wtvdec.c
index 2de6dc2103..4ce4b6403e 100644
--- a/libavformat/wtvdec.c
+++ b/libavformat/wtvdec.c
@@ -468,7 +468,7 @@ static void get_tag(AVFormatContext *s, AVIOContext *pb, const char *key, int ty
         return;
     }
 
-    buf_size = FFMAX(2*length, LEN_PRETTY_GUID) + 1;
+    buf_size = FFMIN(FFMAX(2U * length, LEN_PRETTY_GUID) + 1, INT_MAX);
     buf = av_malloc(buf_size);
     if (!buf)
         return;
-- 
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 4/5] avformat/wtvdec: Use smaller upper bound for buffer

[Andreas Rheinhardt]

Every code point in the BMP is representable with at most three bytes
in UTF-8 and every code point not in the BMP takes four bytes.
For each of the latter, the encoding of UTF-16 takes as many
bytes; for each of the former, it takes at most 3/2 as many.
Therefore one can decrease the size of the buffer allocated
here.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavformat/wtvdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/wtvdec.c b/libavformat/wtvdec.c
index 4ce4b6403e..9fe00590c8 100644
--- a/libavformat/wtvdec.c
+++ b/libavformat/wtvdec.c
@@ -468,7 +468,7 @@ static void get_tag(AVFormatContext *s, AVIOContext *pb, const char *key, int ty
         return;
     }
 
-    buf_size = FFMIN(FFMAX(2U * length, LEN_PRETTY_GUID) + 1, INT_MAX);
+    buf_size = FFMIN(FFMAX(length + length / 2U, LEN_PRETTY_GUID) + 1, INT_MAX);
     buf = av_malloc(buf_size);
     if (!buf)
         return;
-- 
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 5/5] avformat/wtvdec: Avoid unnecessary allocations

[Andreas Rheinhardt]

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

diff --git a/libavformat/wtvdec.c b/libavformat/wtvdec.c
index 9fe00590c8..e70470f79b 100644
--- a/libavformat/wtvdec.c
+++ b/libavformat/wtvdec.c
@@ -460,71 +460,62 @@ done:
 
 static void get_tag(AVFormatContext *s, AVIOContext *pb, const char *key, int type, int length)
 {
-    int buf_size;
-    char *buf;
+    char buf[LEN_PRETTY_GUID + 1], *bufp = buf;
+    unsigned dict_flags = 0;
 
     if (!strcmp(key, "WM/MediaThumbType")) {
         avio_skip(pb, length);
         return;
     }
 
-    buf_size = FFMIN(FFMAX(length + length / 2U, LEN_PRETTY_GUID) + 1, INT_MAX);
-    buf = av_malloc(buf_size);
-    if (!buf)
-        return;
-
     if (type == 0 && length == 4) {
-        snprintf(buf, buf_size, "%u", avio_rl32(pb));
+        snprintf(buf, sizeof(buf), "%u", avio_rl32(pb));
     } else if (type == 1) {
-        avio_get_str16le(pb, length, buf, buf_size);
-        if (!strlen(buf)) {
-           av_free(buf);
+        int buflen = FFMIN(length + length / 2U + 1, INT_MAX);
+        bufp = av_malloc(buflen);
+        if (!bufp)
+            return;
+        avio_get_str16le(pb, length, bufp, buflen);
+        if (!*bufp) {
+           av_free(bufp);
            return;
         }
+        dict_flags = AV_DICT_DONT_STRDUP_VAL;
     } else if (type == 3 && length == 4) {
         strcpy(buf, avio_rl32(pb) ? "true" : "false");
     } else if (type == 4 && length == 8) {
         int64_t num = avio_rl64(pb);
         if (!strcmp(key, "WM/EncodingTime") ||
             !strcmp(key, "WM/MediaOriginalBroadcastDateTime")) {
-            if (filetime_to_iso8601(buf, buf_size, num) < 0) {
-                av_free(buf);
+            if (filetime_to_iso8601(buf, sizeof(buf), num) < 0)
                 return;
-            }
         } else if (!strcmp(key, "WM/WMRVEncodeTime") ||
                    !strcmp(key, "WM/WMRVEndTime")) {
-            if (crazytime_to_iso8601(buf, buf_size, num) < 0) {
-                av_free(buf);
+            if (crazytime_to_iso8601(buf, sizeof(buf), num) < 0)
                 return;
-            }
         } else if (!strcmp(key, "WM/WMRVExpirationDate")) {
-            if (oledate_to_iso8601(buf, buf_size, num) < 0 ) {
-                av_free(buf);
+            if (oledate_to_iso8601(buf, sizeof(buf), num) < 0)
                 return;
-            }
         } else if (!strcmp(key, "WM/WMRVBitrate"))
-            snprintf(buf, buf_size, "%f", av_int2double(num));
+            snprintf(buf, sizeof(buf), "%f", av_int2double(num));
         else
-            snprintf(buf, buf_size, "%"PRIi64, num);
+            snprintf(buf, sizeof(buf), "%"PRIi64, num);
     } else if (type == 5 && length == 2) {
-        snprintf(buf, buf_size, "%u", avio_rl16(pb));
+        snprintf(buf, sizeof(buf), "%u", avio_rl16(pb));
     } else if (type == 6 && length == 16) {
         ff_asf_guid guid;
         avio_read(pb, guid, 16);
-        snprintf(buf, buf_size, PRI_PRETTY_GUID, ARG_PRETTY_GUID(guid));
+        snprintf(buf, sizeof(buf), PRI_PRETTY_GUID, ARG_PRETTY_GUID(guid));
     } else if (type == 2 && !strcmp(key, "WM/Picture")) {
         get_attachment(s, pb, length);
-        av_freep(&buf);
         return;
     } else {
-        av_freep(&buf);
         av_log(s, AV_LOG_WARNING, "unsupported metadata entry; key:%s, type:%d, length:0x%x\n", key, type, length);
         avio_skip(pb, length);
         return;
     }
 
-    av_dict_set(&s->metadata, key, buf, 0);
-    av_freep(&buf);
+    av_dict_set(&s->metadata, key, bufp, dict_flags);
 }
 
 /**
-- 
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/5] avformat/wtvdec: Don't truncate GUIDs

[Peter Ross]

[-- Attachment #1.1: Type: text/plain, Size: 576 bytes --]

On Tue, Sep 12, 2023 at 02:27:17PM +0200, Andreas Rheinhardt wrote:
> Each of the 16 bytes of a GUID is written as a two-character
> hex value and three hyphens, leading to a length of 35.
> GCC 13 emits a -Wformat-truncation= warning because of this.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
> Is there actually a reason this is using a different format than
> the one used by lavu/uuid.h?

i don't think so. wtvdec predates lavu/uuid.h.

patchset looks good.

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)

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