diff mbox series

[FFmpeg-devel,3/3] avcodec/mjpegdec: Fix leak in case ICC array allocations partially fail

Message ID HE1PR0301MB215427498CF5E80C4DF41BC58F799@HE1PR0301MB2154.eurprd03.prod.outlook.com
State Accepted
Commit a5b2f06b0c69221e375edd918a335c68b33d5667
Headers show
Series [FFmpeg-devel,1/3] avcodec/mjpegdec: Fix leak in case of invalid external Huffman tables | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Andreas Rheinhardt April 3, 2021, 2:22 p.m. UTC
If only one of the two arrays used for the ICC profile could be
successfully allocated, it might be overwritten and leak when
the next ICC entry is encountered. Fix this by using a common struct,
so that one has only one array to allocate.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
See https://github.com/drewnoakes/metadata-extractor/issues/65 for a
sample.

 libavcodec/mjpegdec.c | 28 +++++++++++++---------------
 libavcodec/mjpegdec.h |  8 ++++++--
 2 files changed, 19 insertions(+), 17 deletions(-)
diff mbox series

Patch

diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index d0c933b52e..f3d9e99aab 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -2088,28 +2088,26 @@  static int mjpeg_decode_app(MJpegDecodeContext *s)
 
         /* Allocate if this is the first APP2 we've seen. */
         if (s->iccnum == 0) {
-            s->iccdata     = av_mallocz(nummarkers * sizeof(*(s->iccdata)));
-            s->iccdatalens = av_mallocz(nummarkers * sizeof(*(s->iccdatalens)));
-            if (!s->iccdata || !s->iccdatalens) {
+            if (!FF_ALLOCZ_TYPED_ARRAY(s->iccentries, nummarkers)) {
                 av_log(s->avctx, AV_LOG_ERROR, "Could not allocate ICC data arrays\n");
                 return AVERROR(ENOMEM);
             }
             s->iccnum = nummarkers;
         }
 
-        if (s->iccdata[seqno - 1]) {
+        if (s->iccentries[seqno - 1].data) {
             av_log(s->avctx, AV_LOG_WARNING, "Duplicate ICC sequence number\n");
             goto out;
         }
 
-        s->iccdatalens[seqno - 1]  = len;
-        s->iccdata[seqno - 1]      = av_malloc(len);
-        if (!s->iccdata[seqno - 1]) {
+        s->iccentries[seqno - 1].length = len;
+        s->iccentries[seqno - 1].data   = av_malloc(len);
+        if (!s->iccentries[seqno - 1].data) {
             av_log(s->avctx, AV_LOG_ERROR, "Could not allocate ICC data buffer\n");
             return AVERROR(ENOMEM);
         }
 
-        memcpy(s->iccdata[seqno - 1], align_get_bits(&s->gb), len);
+        memcpy(s->iccentries[seqno - 1].data, align_get_bits(&s->gb), len);
         skip_bits(&s->gb, len << 3);
         len = 0;
         s->iccread++;
@@ -2318,11 +2316,11 @@  static void reset_icc_profile(MJpegDecodeContext *s)
 {
     int i;
 
-    if (s->iccdata)
+    if (s->iccentries) {
         for (i = 0; i < s->iccnum; i++)
-            av_freep(&s->iccdata[i]);
-    av_freep(&s->iccdata);
-    av_freep(&s->iccdatalens);
+            av_freep(&s->iccentries[i].data);
+        av_freep(&s->iccentries);
+    }
 
     s->iccread = 0;
     s->iccnum  = 0;
@@ -2838,7 +2836,7 @@  the_end:
 
         /* Sum size of all parts. */
         for (i = 0; i < s->iccnum; i++)
-            total_size += s->iccdatalens[i];
+            total_size += s->iccentries[i].length;
 
         sd = av_frame_new_side_data(frame, AV_FRAME_DATA_ICC_PROFILE, total_size);
         if (!sd) {
@@ -2848,8 +2846,8 @@  the_end:
 
         /* Reassemble the parts, which are now in-order. */
         for (i = 0; i < s->iccnum; i++) {
-            memcpy(sd->data + offset, s->iccdata[i], s->iccdatalens[i]);
-            offset += s->iccdatalens[i];
+            memcpy(sd->data + offset, s->iccentries[i].data, s->iccentries[i].length);
+            offset += s->iccentries[i].length;
         }
     }
 
diff --git a/libavcodec/mjpegdec.h b/libavcodec/mjpegdec.h
index 732aeab994..0d69d9101b 100644
--- a/libavcodec/mjpegdec.h
+++ b/libavcodec/mjpegdec.h
@@ -44,6 +44,11 @@ 
 
 #define MAX_COMPONENTS 4
 
+typedef struct ICCEntry {
+    uint8_t *data;
+    int    length;
+} ICCEntry;
+
 typedef struct MJpegDecodeContext {
     AVClass *class;
     AVCodecContext *avctx;
@@ -138,8 +143,7 @@  typedef struct MJpegDecodeContext {
 
     const AVPixFmtDescriptor *pix_desc;
 
-    uint8_t **iccdata;
-    int *iccdatalens;
+    ICCEntry *iccentries;
     int iccnum;
     int iccread;