[vlc-commits] demux/xiph_metadata: fix overflow/leaks in vorbis_ParseComment

[Filip Roséen]

vlc/vlc-2.2 | branch: master | Filip Roséen <filip at atch.se> | Wed Dec 14 15:51:06 2016 +0100| [9745e9d0f2d0749afc971dc311cacc4a9acdcab6] | committer: Jean-Baptiste Kempf

demux/xiph_metadata: fix overflow/leaks in vorbis_ParseComment

The previous implementation would suffer from a read overflow due to a
mismatch between the length of psz_comment and comment_size (because
of the usage of strndup).

These changes make sure that:

 - psz_comment always refer to a buffer of length comment_size
 - we do not leak memory on "continues" when encountering unexpected
   data

fixes #17776
fixes #17779

Signed-off-by: Francois Cartegnie <fcvlcdev at free.fr>
(cherry picked from commit e8dde09cac172b5ae4be9266575b6615e315b801)
Signed-off-by: Jean-Baptiste Kempf <jb at videolan.org>

> http://git.videolan.org/gitweb.cgi/vlc/vlc-2.2.git/?a=commit;h=9745e9d0f2d0749afc971dc311cacc4a9acdcab6
---

 modules/demux/xiph_metadata.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/modules/demux/xiph_metadata.c b/modules/demux/xiph_metadata.c
index 0d074f0..4175c56 100644
--- a/modules/demux/xiph_metadata.c
+++ b/modules/demux/xiph_metadata.c
@@ -250,9 +250,13 @@ void vorbis_ParseComment( es_format_t *p_fmt, vlc_meta_t **pp_meta,
         if( comment_size == 0 )
             continue;
 
-        char* psz_comment = strndup( (const char*)p_data, comment_size );
+        char* psz_comment = malloc( comment_size + 1 );
 
-        RM(comment_size);
+        if( unlikely( !psz_comment ) )
+            goto next_comment;
+
+        memcpy( psz_comment, p_data, comment_size );
+        psz_comment[comment_size] = '\0';
 
         EnsureUTF8( psz_comment );
 
@@ -327,7 +331,7 @@ void vorbis_ParseComment( es_format_t *p_fmt, vlc_meta_t **pp_meta,
         else if( !strncasecmp( psz_comment, "METADATA_BLOCK_PICTURE=", strlen("METADATA_BLOCK_PICTURE=")))
         {
             if( attachments == NULL )
-                continue;
+                goto next_comment;
 
             uint8_t *p_picture;
             size_t i_size = vlc_b64_decode_binary( &p_picture, &psz_comment[strlen("METADATA_BLOCK_PICTURE=")]);
@@ -343,8 +347,9 @@ void vorbis_ParseComment( es_format_t *p_fmt, vlc_meta_t **pp_meta,
         else if ( ppf_replay_gain && ppf_replay_peak && !strncmp(psz_comment, "REPLAYGAIN_", 11) )
         {
             char *p = strchr( psz_comment, '=' );
+            if (!p) goto next_comment;
+
             char *psz_val;
-            if (!p) continue;
             if ( !strncasecmp(psz_comment, "REPLAYGAIN_TRACK_GAIN=", 22) )
             {
                 psz_val = malloc( strlen(p+1) + 1 );
@@ -388,7 +393,7 @@ void vorbis_ParseComment( es_format_t *p_fmt, vlc_meta_t **pp_meta,
             {
                 char *p = strchr( psz_comment, '=' );
                 p_seekpoint = getChapterEntry( i_chapt, &chapters_array );
-                if ( !p || ! p_seekpoint ) continue;
+                if ( !p || ! p_seekpoint ) goto next_comment;
                 if ( ! p_seekpoint->psz_name )
                     p_seekpoint->psz_name = strdup( ++p );
             }
@@ -399,7 +404,7 @@ void vorbis_ParseComment( es_format_t *p_fmt, vlc_meta_t **pp_meta,
                 if( p && sscanf( ++p, "%u:%u:%u.%u", &h, &m, &s, &ms ) == 4 )
                 {
                     p_seekpoint = getChapterEntry( i_chapt, &chapters_array );
-                    if ( ! p_seekpoint ) continue;
+                    if ( ! p_seekpoint ) goto next_comment;
                     p_seekpoint->i_time_offset =
                       (((int64_t)h * 3600 + (int64_t)m * 60 + (int64_t)s) * 1000 + ms) * 1000;
                 }
@@ -419,7 +424,9 @@ void vorbis_ParseComment( es_format_t *p_fmt, vlc_meta_t **pp_meta,
             vlc_meta_AddExtra( p_meta, psz_comment, p );
         }
 #undef IF_EXTRACT
+next_comment:
         free( psz_comment );
+        RM( comment_size );
     }
 #undef RM