[FFmpeg-devel] [PATCH] avformat/mpegts: Don't leave context in inconsistent state upon error

[Andreas Rheinhardt]

Up until now, opening a section filter works as follows: A filter is
opened and (on success) attached to the MpegTSContext. Then a buffer for
said filter is allocated and upon success attached to the section
filter; on error, the filter is simply freed without removing it from
the MpegTSContext, leaving the latter in an inconsistent state. This
leads to use-after-frees lateron.

This commit fixes this by allocating the buffer first; the filter is
only opened if the buffer could be successfully allocated.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
---
At first I wanted to use a flexible array member for this buffer (do we
actually support any compiler that doesn't support flexible array
members?), yet for some reason unknown to me a structure containing a
flexible array member must not be contained in another structure or
union (not even at the end, although it makes sense to do so). GCC gives
a warning in pedantic mode for this.

 libavformat/mpegts.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
index c6fd3e1cef..f71f18a57d 100644
--- a/libavformat/mpegts.c
+++ b/libavformat/mpegts.c
@@ -510,20 +510,22 @@ static MpegTSFilter *mpegts_open_section_filter(MpegTSContext *ts,
 {
     MpegTSFilter *filter;
     MpegTSSectionFilter *sec;
+    uint8_t *section_buf = av_mallocz(MAX_SECTION_SIZE);
 
-    if (!(filter = mpegts_open_filter(ts, pid, MPEGTS_SECTION)))
+    if (!section_buf)
         return NULL;
+
+    if (!(filter = mpegts_open_filter(ts, pid, MPEGTS_SECTION))) {
+        av_free(section_buf);
+        return NULL;
+    }
     sec = &filter->u.section_filter;
     sec->section_cb  = section_cb;
     sec->opaque      = opaque;
-    sec->section_buf = av_mallocz(MAX_SECTION_SIZE);
+    sec->section_buf = section_buf;
     sec->check_crc   = check_crc;
     sec->last_ver    = -1;
 
-    if (!sec->section_buf) {
-        av_free(filter);
-        return NULL;
-    }
     return filter;
 }
 
-- 
2.20.1