avformat/flac_picture: Switch to bytestream2 API
authorAndreas Rheinhardt <andreas.rheinhardt@gmail.com>
Fri, 29 Nov 2019 19:44:10 +0000 (20:44 +0100)
committerMichael Niedermayer <michael@niedermayer.cc>
Sun, 1 Dec 2019 16:17:04 +0000 (17:17 +0100)
ff_flac_parse_picture() parses a buffer containing a flac metadata
picture block by wrapping it in an AVIOContext and using the AVIOContext
API. Consequently, when not enough data could be read AVERROR(EIO) was
returned although reading didn't really fail: A block that contains a
subfield whose size field indicates that it is so big as to extend
beyond the buffer is just invalid.

This commit changes this by using the bytestream2 API instead;
furthermore, the checks for whether there is enough data left are
performed before allocating a buffer for said data.

Finally, if the length of the picture description is bigger than
INT_MAX, it will now raise an error.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
libavformat/flac_picture.c

index 6463a37..ccb0ee6 100644 (file)
  */
 
 #include "libavutil/intreadwrite.h"
  */
 
 #include "libavutil/intreadwrite.h"
+#include "libavcodec/bytestream.h"
 #include "libavcodec/png.h"
 #include "avformat.h"
 #include "flac_picture.h"
 #include "id3v2.h"
 #include "internal.h"
 #include "libavcodec/png.h"
 #include "avformat.h"
 #include "flac_picture.h"
 #include "id3v2.h"
 #include "internal.h"
-#include "avio_internal.h"
 
 int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size)
 {
 
 int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size)
 {
@@ -33,16 +33,22 @@ int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size)
     enum AVCodecID id = AV_CODEC_ID_NONE;
     AVBufferRef *data = NULL;
     uint8_t mimetype[64], *desc = NULL;
     enum AVCodecID id = AV_CODEC_ID_NONE;
     AVBufferRef *data = NULL;
     uint8_t mimetype[64], *desc = NULL;
-    AVIOContext pb0, *pb = &pb0;
+    GetByteContext g;
     AVStream *st;
     int width, height, ret = 0;
     AVStream *st;
     int width, height, ret = 0;
-    int len;
-    unsigned int type;
+    unsigned int len, type;
 
 
-    ffio_init_context(pb, buf, buf_size, 0, NULL, NULL, NULL, NULL);
+    if (buf_size < 34) {
+        av_log(s, AV_LOG_ERROR, "Attached picture metadata block too short\n");
+        if (s->error_recognition & AV_EF_EXPLODE)
+            return AVERROR_INVALIDDATA;
+        return 0;
+    }
+
+    bytestream2_init(&g, buf, buf_size);
 
     /* read the picture type */
 
     /* read the picture type */
-    type = avio_rb32(pb);
+    type = bytestream2_get_be32u(&g);
     if (type >= FF_ARRAY_ELEMS(ff_id3v2_picture_types)) {
         av_log(s, AV_LOG_ERROR, "Invalid picture type: %d.\n", type);
         if (s->error_recognition & AV_EF_EXPLODE) {
     if (type >= FF_ARRAY_ELEMS(ff_id3v2_picture_types)) {
         av_log(s, AV_LOG_ERROR, "Invalid picture type: %d.\n", type);
         if (s->error_recognition & AV_EF_EXPLODE) {
@@ -52,15 +58,21 @@ int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size)
     }
 
     /* picture mimetype */
     }
 
     /* picture mimetype */
-    len = avio_rb32(pb);
-    if (len <= 0 || len >= sizeof(mimetype) ||
-        avio_read(pb, mimetype, len) != len) {
+    len = bytestream2_get_be32u(&g);
+    if (len <= 0 || len >= sizeof(mimetype)) {
         av_log(s, AV_LOG_ERROR, "Could not read mimetype from an attached "
                "picture.\n");
         if (s->error_recognition & AV_EF_EXPLODE)
             ret = AVERROR_INVALIDDATA;
         goto fail;
     }
         av_log(s, AV_LOG_ERROR, "Could not read mimetype from an attached "
                "picture.\n");
         if (s->error_recognition & AV_EF_EXPLODE)
             ret = AVERROR_INVALIDDATA;
         goto fail;
     }
+    if (len + 24 > bytestream2_get_bytes_left(&g)) {
+        av_log(s, AV_LOG_ERROR, "Attached picture metadata block too short\n");
+        if (s->error_recognition & AV_EF_EXPLODE)
+            return AVERROR_INVALIDDATA;
+        return 0;
+    }
+    bytestream2_get_bufferu(&g, mimetype, len);
     mimetype[len] = 0;
 
     while (mime->id != AV_CODEC_ID_NONE) {
     mimetype[len] = 0;
 
     while (mime->id != AV_CODEC_ID_NONE) {
@@ -79,30 +91,31 @@ int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size)
     }
 
     /* picture description */
     }
 
     /* picture description */
-    len = avio_rb32(pb);
+    len = bytestream2_get_be32u(&g);
+    if (len > bytestream2_get_bytes_left(&g) - 20) {
+        av_log(s, AV_LOG_ERROR, "Attached picture metadata block too short\n");
+        if (s->error_recognition & AV_EF_EXPLODE)
+            return AVERROR_INVALIDDATA;
+        return 0;
+    }
     if (len > 0) {
         if (!(desc = av_malloc(len + 1))) {
             RETURN_ERROR(AVERROR(ENOMEM));
         }
 
     if (len > 0) {
         if (!(desc = av_malloc(len + 1))) {
             RETURN_ERROR(AVERROR(ENOMEM));
         }
 
-        if (avio_read(pb, desc, len) != len) {
-            av_log(s, AV_LOG_ERROR, "Error reading attached picture description.\n");
-            if (s->error_recognition & AV_EF_EXPLODE)
-                ret = AVERROR(EIO);
-            goto fail;
-        }
+        bytestream2_get_bufferu(&g, desc, len);
         desc[len] = 0;
     }
 
     /* picture metadata */
         desc[len] = 0;
     }
 
     /* picture metadata */
-    width  = avio_rb32(pb);
-    height = avio_rb32(pb);
-    avio_skip(pb, 8);
+    width  = bytestream2_get_be32u(&g);
+    height = bytestream2_get_be32u(&g);
+    bytestream2_skipu(&g, 8);
 
     /* picture data */
 
     /* picture data */
-    len = avio_rb32(pb);
-    if (len <= 0) {
-        av_log(s, AV_LOG_ERROR, "Invalid attached picture size: %d.\n", len);
+    len = bytestream2_get_be32u(&g);
+    if (len <= 0 || len > bytestream2_get_bytes_left(&g)) {
+        av_log(s, AV_LOG_ERROR, "Attached picture metadata block too short\n");
         if (s->error_recognition & AV_EF_EXPLODE)
             ret = AVERROR_INVALIDDATA;
         goto fail;
         if (s->error_recognition & AV_EF_EXPLODE)
             ret = AVERROR_INVALIDDATA;
         goto fail;
@@ -110,13 +123,8 @@ int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size)
     if (!(data = av_buffer_alloc(len + AV_INPUT_BUFFER_PADDING_SIZE))) {
         RETURN_ERROR(AVERROR(ENOMEM));
     }
     if (!(data = av_buffer_alloc(len + AV_INPUT_BUFFER_PADDING_SIZE))) {
         RETURN_ERROR(AVERROR(ENOMEM));
     }
+    bytestream2_get_bufferu(&g, data->data, len);
     memset(data->data + len, 0, AV_INPUT_BUFFER_PADDING_SIZE);
     memset(data->data + len, 0, AV_INPUT_BUFFER_PADDING_SIZE);
-    if (avio_read(pb, data->data, len) != len) {
-        av_log(s, AV_LOG_ERROR, "Error reading attached picture data.\n");
-        if (s->error_recognition & AV_EF_EXPLODE)
-            ret = AVERROR(EIO);
-        goto fail;
-    }
 
     if (AV_RB64(data->data) == PNGSIG)
         id = AV_CODEC_ID_PNG;
 
     if (AV_RB64(data->data) == PNGSIG)
         id = AV_CODEC_ID_PNG;