[FFmpeg-devel] [PATCH 3/4] avutil/frame: Reimplement av_frame_new_side_data() without size=0 special case

[Michael Niedermayer]

The size 0 special case causes side data to be created which is
different and a special case if for any reasons size = 0 is passed

Fixes: multiple runtime error: null pointer passed as argument 1, which is declared to never be null
Fixes: 653/clusterfuzz-testcase-5773837415219200

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg

Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
---
 libavutil/frame.c | 55 +++++++++++++++++++++++++++++--------------------------
 1 file changed, 29 insertions(+), 26 deletions(-)

diff --git a/libavutil/frame.c b/libavutil/frame.c
index 2913982e91..8811dcdcfe 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -26,6 +26,11 @@
 #include "mem.h"
 #include "samplefmt.h"
 
+
+static AVFrameSideData *frame_new_side_data(AVFrame *frame,
+                                            enum AVFrameSideDataType type,
+                                            AVBufferRef *buf);
+
 MAKE_ACCESSORS(AVFrame, frame, int64_t, best_effort_timestamp)
 MAKE_ACCESSORS(AVFrame, frame, int64_t, pkt_duration)
 MAKE_ACCESSORS(AVFrame, frame, int64_t, pkt_pos)
@@ -344,20 +349,11 @@ FF_ENABLE_DEPRECATION_WARNINGS
             }
             memcpy(sd_dst->data, sd_src->data, sd_src->size);
         } else {
-            sd_dst = av_frame_new_side_data(dst, sd_src->type, 0);
+            sd_dst = frame_new_side_data(dst, sd_src->type, av_buffer_ref(sd_src->buf));
             if (!sd_dst) {
                 wipe_side_data(dst);
                 return AVERROR(ENOMEM);
             }
-            if (sd_src->buf) {
-                sd_dst->buf = av_buffer_ref(sd_src->buf);
-                if (!sd_dst->buf) {
-                    wipe_side_data(dst);
-                    return AVERROR(ENOMEM);
-                }
-                sd_dst->data = sd_dst->buf->data;
-                sd_dst->size = sd_dst->buf->size;
-            }
         }
         av_dict_copy(&sd_dst->metadata, sd_src->metadata, 0);
     }
@@ -633,40 +629,47 @@ AVBufferRef *av_frame_get_plane_buffer(AVFrame *frame, int plane)
     return NULL;
 }
 
-AVFrameSideData *av_frame_new_side_data(AVFrame *frame,
-                                        enum AVFrameSideDataType type,
-                                        int size)
+static AVFrameSideData *frame_new_side_data(AVFrame *frame,
+                                            enum AVFrameSideDataType type,
+                                            AVBufferRef *buf)
 {
     AVFrameSideData *ret, **tmp;
 
-    if (frame->nb_side_data > INT_MAX / sizeof(*frame->side_data) - 1)
+    if (!buf)
         return NULL;
 
+    if (frame->nb_side_data > INT_MAX / sizeof(*frame->side_data) - 1)
+        goto fail;
+
     tmp = av_realloc(frame->side_data,
                      (frame->nb_side_data + 1) * sizeof(*frame->side_data));
     if (!tmp)
-        return NULL;
+        goto fail;
     frame->side_data = tmp;
 
     ret = av_mallocz(sizeof(*ret));
     if (!ret)
-        return NULL;
-
-    if (size > 0) {
-        ret->buf = av_buffer_alloc(size);
-        if (!ret->buf) {
-            av_freep(&ret);
-            return NULL;
-        }
+        goto fail;
 
-        ret->data = ret->buf->data;
-        ret->size = size;
-    }
+    ret->buf = buf;
+    ret->data = ret->buf->data;
+    ret->size = buf->size;
     ret->type = type;
 
     frame->side_data[frame->nb_side_data++] = ret;
 
     return ret;
+fail:
+    av_buffer_unref(&buf);
+    return NULL;
+}
+
+AVFrameSideData *av_frame_new_side_data(AVFrame *frame,
+                                        enum AVFrameSideDataType type,
+                                        int size)
+{
+
+    return frame_new_side_data(frame, type, av_buffer_alloc(size));
 }
 
 AVFrameSideData *av_frame_get_side_data(const AVFrame *frame,
-- 
2.11.0