[FFmpeg-devel] [PATCH 1/6] avcodec/tiff: Fix handling of av_strdup() failures

* [FFmpeg-devel] [PATCH 1/6] avcodec/tiff: Fix handling of av_strdup() failures
@ 2024-03-10 14:12 Andreas Rheinhardt
  2024-03-10 14:15 ` [FFmpeg-devel] [PATCH 2/6] avcodec/tiff: Avoid duplicating strings Andreas Rheinhardt
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Andreas Rheinhardt @ 2024-03-10 14:12 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Andreas Rheinhardt

For unknown geokey values, get_geokey_val() returns
"Unknown-%d" with val being used for %d. This string
is allocated and therefore all the known geokey values
(static strings) are strdup'ed. In case this fails
it is either ignored or treated as "Unknown-%d".
(Furthermore it is possible to call av_strdup(NULL),
although this is not documented to be legal.)

This commit changes this by only returning the static strings
in get_geokey_val(); the unknown handling and strdup'ing
is moved out of it.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/tiff.c | 35 ++++++++++++++++-------------------
 1 file changed, 16 insertions(+), 19 deletions(-)

diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c
index cb4d378753..4c7460cf41 100644
--- a/libavcodec/tiff.c
+++ b/libavcodec/tiff.c
@@ -36,6 +36,7 @@
 #include <float.h>
 
 #include "libavutil/attributes.h"
+#include "libavutil/avstring.h"
 #include "libavutil/error.h"
 #include "libavutil/intreadwrite.h"
 #include "libavutil/opt.h"
@@ -179,19 +180,17 @@ static const char *search_keyval(const TiffGeoTagKeyName *keys, int n, int id)
     return NULL;
 }
 
-static char *get_geokey_val(int key, int val)
+static const char *get_geokey_val(int key, uint16_t val)
 {
-    char *ap;
-
     if (val == TIFF_GEO_KEY_UNDEFINED)
-        return av_strdup("undefined");
+        return "undefined";
     if (val == TIFF_GEO_KEY_USER_DEFINED)
-        return av_strdup("User-Defined");
+        return "User-Defined";
 
 #define RET_GEOKEY_VAL(TYPE, array)\
     if (val >= TIFF_##TYPE##_OFFSET &&\
         val - TIFF_##TYPE##_OFFSET < FF_ARRAY_ELEMS(tiff_##array##_codes))\
-        return av_strdup(tiff_##array##_codes[val - TIFF_##TYPE##_OFFSET]);
+        return tiff_##array##_codes[val - TIFF_##TYPE##_OFFSET];
 
     switch (key) {
     case TIFF_GT_MODEL_TYPE_GEOKEY:
@@ -224,13 +223,9 @@ static char *get_geokey_val(int key, int val)
         RET_GEOKEY_VAL(PRIME_MERIDIAN, prime_meridian);
         break;
     case TIFF_PROJECTED_CS_TYPE_GEOKEY:
-        ap = av_strdup(search_keyval(tiff_proj_cs_type_codes, FF_ARRAY_ELEMS(tiff_proj_cs_type_codes), val));
-        if(ap) return ap;
-        break;
+        return search_keyval(tiff_proj_cs_type_codes, FF_ARRAY_ELEMS(tiff_proj_cs_type_codes), val);
     case TIFF_PROJECTION_GEOKEY:
-        ap = av_strdup(search_keyval(tiff_projection_codes, FF_ARRAY_ELEMS(tiff_projection_codes), val));
-        if(ap) return ap;
-        break;
+        return search_keyval(tiff_projection_codes, FF_ARRAY_ELEMS(tiff_projection_codes), val);
     case TIFF_PROJ_COORD_TRANS_GEOKEY:
         RET_GEOKEY_VAL(COORD_TRANS, coord_trans);
         break;
@@ -241,10 +236,7 @@ static char *get_geokey_val(int key, int val)
 
     }
 
-    ap = av_malloc(14);
-    if (ap)
-        snprintf(ap, 14, "Unknown-%d", val);
-    return ap;
+    return NULL;
 }
 
 static char *doubles2str(double *dp, int count, const char *sep)
@@ -1634,9 +1626,14 @@ static int tiff_decode_tag(TiffContext *s, AVFrame *frame)
             s->geotags[i].type   = ff_tget_short(&s->gb, s->le);
             s->geotags[i].count  = ff_tget_short(&s->gb, s->le);
 
-            if (!s->geotags[i].type)
-                s->geotags[i].val  = get_geokey_val(s->geotags[i].key, ff_tget_short(&s->gb, s->le));
-            else
+            if (!s->geotags[i].type) {
+                uint16_t val    = ff_tget_short(&s->gb, s->le);
+                const char *str = get_geokey_val(s->geotags[i].key, val);
+
+                s->geotags[i].val = str ? av_strdup(str) : av_asprintf("Unknown-%u", val);
+                if (!s->geotags[i].val)
+                    return AVERROR(ENOMEM);
+            } else
                 s->geotags[i].offset = ff_tget_short(&s->gb, s->le);
         }
         break;
-- 
2.40.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".

^ permalink raw reply	[flat|nested] 13+ messages in thread