[vlc-commits] subsdec: really fix buffer overflows

Rémi Denis-Courmont git at videolan.org
Sat Nov 17 19:05:50 CET 2012


vlc/vlc-2.0 | branch: master | Rémi Denis-Courmont <remi at remlab.net> | Sat Nov 17 20:00:34 2012 +0200| [8e8b02ff1720eb46dabe2864e79d47b40a2792d5] | committer: Rémi Denis-Courmont

subsdec: really fix buffer overflows

Reported-by: Aliz Hammond
(cherry picked from commit ee86514d1cb1fe5ec25c8bdfb2a4b6a8d98591e7)

> http://git.videolan.org/gitweb.cgi/vlc/vlc-2.0.git/?a=commit;h=8e8b02ff1720eb46dabe2864e79d47b40a2792d5
---

 modules/codec/subsdec.c |  158 +++++++++++++++++++++--------------------------
 1 file changed, 70 insertions(+), 88 deletions(-)

diff --git a/modules/codec/subsdec.c b/modules/codec/subsdec.c
index e44190b..ed4d1f7 100644
--- a/modules/codec/subsdec.c
+++ b/modules/codec/subsdec.c
@@ -31,6 +31,8 @@
 # include "config.h"
 #endif
 
+#include <limits.h>
+
 #include <vlc_common.h>
 #include <vlc_plugin.h>
 #include <vlc_codec.h>
@@ -574,19 +576,32 @@ static char *StripTags( char *psz_subtitle )
  * returned, and the rendering engine will fall back to the
  * plain text version of the subtitle.
  */
+/* TODO: highly suboptimal, offset should be cached */
 static void HtmlNPut( char **ppsz_html, const char *psz_text, int i_max )
 {
-    const int i_len = strlen(psz_text);
+    char *psz_html = *ppsz_html;
+    if( psz_html == NULL )
+        return;
+
+    const size_t i_offset = strlen(psz_html);
+    const size_t i_len = strnlen(psz_text, i_max);
 
-    strncpy( *ppsz_html, psz_text, i_max );
-    *ppsz_html += __MIN(i_max,i_len);
+    psz_html = realloc( psz_html, i_offset + i_len + 1 );
+    if( psz_html != NULL )
+    {
+        memcpy( psz_html + i_offset, psz_text, i_len );
+        psz_html[i_offset + i_len] = '\0';
+    }
+    else
+        free( *ppsz_html );
+    *ppsz_html = psz_html;
 }
 
 static void HtmlPut( char **ppsz_html, const char *psz_text )
 {
-    strcpy( *ppsz_html, psz_text );
-    *ppsz_html += strlen(psz_text);
+    HtmlNPut( ppsz_html, psz_text, INT_MAX );
 }
+
 static void HtmlCopy( char **ppsz_html, char **ppsz_subtitle, const char *psz_text )
 {
     HtmlPut( ppsz_html, psz_text );
@@ -595,22 +610,17 @@ static void HtmlCopy( char **ppsz_html, char **ppsz_subtitle, const char *psz_te
 
 static char *CreateHtmlSubtitle( int *pi_align, char *psz_subtitle )
 {
-    /* */
-    char *psz_tag = malloc( ( strlen( psz_subtitle ) / 3 ) + 1 );
-    if( !psz_tag )
+    char *psz_tag = malloc( 1 );
+    if( psz_tag == NULL )
         return NULL;
-    psz_tag[ 0 ] = '\0';
 
-    /* */
-    //Oo + 100 ???
-    size_t i_buf_size = strlen( psz_subtitle ) + 100;
-    char   *psz_html_start = malloc( i_buf_size );
-    char   *psz_html = psz_html_start;
-    if( psz_html_start == NULL )
+    char *psz_html = malloc( 1 );
+    if( psz_html == NULL )
     {
         free( psz_tag );
         return NULL;
     }
+    psz_tag[0] = '\0';
     psz_html[0] = '\0';
 
     bool b_has_align = false;
@@ -634,22 +644,22 @@ static char *CreateHtmlSubtitle( int *pi_align, char *psz_subtitle )
             else if( !strncasecmp( psz_subtitle, "<b>", 3 ) )
             {
                 HtmlCopy( &psz_html, &psz_subtitle, "<b>" );
-                strcat( psz_tag, "b" );
+                HtmlPut( &psz_tag, "b" );
             }
             else if( !strncasecmp( psz_subtitle, "<i>", 3 ) )
             {
                 HtmlCopy( &psz_html, &psz_subtitle, "<i>" );
-                strcat( psz_tag, "i" );
+                HtmlPut( &psz_tag, "i" );
             }
             else if( !strncasecmp( psz_subtitle, "<u>", 3 ) )
             {
                 HtmlCopy( &psz_html, &psz_subtitle, "<u>" );
-                strcat( psz_tag, "u" );
+                HtmlPut( &psz_tag, "u" );
             }
             else if( !strncasecmp( psz_subtitle, "<s>", 3 ) )
             {
                 HtmlCopy( &psz_html, &psz_subtitle, "<s>" );
-                strcat( psz_tag, "s" );
+                HtmlPut( &psz_tag, "s" );
             }
             else if( !strncasecmp( psz_subtitle, "<font ", 6 ))
             {
@@ -659,7 +669,7 @@ static char *CreateHtmlSubtitle( int *pi_align, char *psz_subtitle )
                         "alpha=", NULL };
 
                 HtmlCopy( &psz_html, &psz_subtitle, "<font " );
-                strcat( psz_tag, "f" );
+                HtmlPut( &psz_tag, "f" );
 
                 /* <font       color= */
                 while (*psz_subtitle == ' ')
@@ -725,10 +735,9 @@ static char *CreateHtmlSubtitle( int *pi_align, char *psz_subtitle )
                         psz_subtitle += i_len;
                     }
 
-                    while (*psz_subtitle == ' ')
-                        *psz_html++ = *psz_subtitle++;
+                    HtmlNPut( &psz_html, psz_subtitle, strspn(psz_subtitle, " ") );
                 }
-                *psz_html++ = '>';
+                HtmlPut( &psz_html, ">" );
                 psz_subtitle++;
             }
             else if( !strncmp( psz_subtitle, "</", 2 ))
@@ -777,8 +786,8 @@ static char *CreateHtmlSubtitle( int *pi_align, char *psz_subtitle )
                 if( !b_match )
                 {
                     /* Not well formed -- kill everything */
-                    free( psz_html_start );
-                    psz_html_start = NULL;
+                    free( psz_html );
+                    psz_html = NULL;
                     break;
                 }
                 *psz_lastTag = '\0';
@@ -818,7 +827,7 @@ static char *CreateHtmlSubtitle( int *pi_align, char *psz_subtitle )
                     {
                         /* We have the closing tag, ignore it TODO */
                         psz_subtitle = &psz_stop[1];
-                        strcat( psz_tag, "I" );
+                        HtmlPut( &psz_tag, "I" );
                     }
                     else
                     {
@@ -832,7 +841,7 @@ static char *CreateHtmlSubtitle( int *pi_align, char *psz_subtitle )
                             else if( *psz_subtitle == '>' )
                                 HtmlPut( &psz_html, ">" );
                             else
-                                *psz_html++ = *psz_subtitle;
+                                HtmlNPut( &psz_html, psz_subtitle, 1 );
                         }
                     }
                 }
@@ -896,17 +905,17 @@ static char *CreateHtmlSubtitle( int *pi_align, char *psz_subtitle )
             if( psz_subtitle[3] == 'i' )
             {
                 HtmlPut( &psz_html, "<i>" );
-                strcat( psz_tag, "i" );
+                HtmlPut( &psz_tag, "i" );
             }
             if( psz_subtitle[3] == 'b' )
             {
                 HtmlPut( &psz_html, "<b>" );
-                strcat( psz_tag, "b" );
+                HtmlPut( &psz_tag, "b" );
             }
             if( psz_subtitle[3] == 'u' )
             {
                 HtmlPut( &psz_html, "<u>" );
-                strcat( psz_tag, "u" );
+                HtmlPut( &psz_tag, "u" );
             }
             psz_subtitle = strchr( psz_subtitle, '}' ) + 1;
         }
@@ -936,10 +945,12 @@ static char *CreateHtmlSubtitle( int *pi_align, char *psz_subtitle )
         }
         else
         {
-            *psz_html = *psz_subtitle;
-            if( psz_html > psz_html_start )
+            HtmlNPut( &psz_html, psz_subtitle, 1 );
+#if 0
+            if( *psz_html )
             {
                 /* Check for double whitespace */
+# error This test does not make sense.
                 if( ( *psz_html == ' '  || *psz_html == '\t' ) &&
                     ( *(psz_html-1) == ' ' || *(psz_html-1) == '\t' ) )
                 {
@@ -947,70 +958,41 @@ static char *CreateHtmlSubtitle( int *pi_align, char *psz_subtitle )
                     psz_html--;
                 }
             }
-            psz_html++;
+#endif
             psz_subtitle++;
         }
+    }
 
-        if( ( size_t )( psz_html - psz_html_start ) > i_buf_size - 50 )
+    while( *psz_tag )
+    {
+        /* */
+        char *psz_last = &psz_tag[strlen(psz_tag)-1];
+        switch( *psz_last )
         {
-            const int i_len = psz_html - psz_html_start;
-
-            i_buf_size += 200;
-            char *psz_new = realloc( psz_html_start, i_buf_size );
-            if( !psz_new )
+            case 'b':
+                HtmlPut( &psz_html, "</b>" );
+                break;
+            case 'i':
+                HtmlPut( &psz_html, "</i>" );
+                break;
+            case 'u':
+                HtmlPut( &psz_html, "</u>" );
+                break;
+            case 's':
+                HtmlPut( &psz_html, "</s>" );
+                break;
+            case 'f':
+                HtmlPut( &psz_html, "</font>" );
+                break;
+            case 'I':
                 break;
-            psz_html_start = psz_new;
-            psz_html = &psz_new[i_len];
         }
+        *psz_last = '\0';
     }
-    if( psz_html_start )
-    {
-        static const char *psz_text_close = "</text>";
-        static const char *psz_tag_long = "/font>";
+    /* Close not well formed subtitle */
+    HtmlPut( &psz_html, "</text>" );
 
-        /* Realloc for closing tags and shrink memory */
-        const size_t i_length = (size_t)( psz_html - psz_html_start );
-
-        const size_t i_size = i_length + strlen(psz_tag_long) * strlen(psz_tag) + strlen(psz_text_close) + 1;
-        char *psz_new = realloc( psz_html_start, i_size );
-        if( psz_new )
-        {
-            psz_html_start = psz_new;
-            psz_html = &psz_new[i_length];
-
-            /* Close not well formed subtitle */
-            while( *psz_tag )
-            {
-                /* */
-                char *psz_last = &psz_tag[strlen(psz_tag)-1];
-                switch( *psz_last )
-                {
-                case 'b':
-                    HtmlPut( &psz_html, "</b>" );
-                    break;
-                case 'i':
-                    HtmlPut( &psz_html, "</i>" );
-                    break;
-                case 'u':
-                    HtmlPut( &psz_html, "</u>" );
-                    break;
-                case 's':
-                    HtmlPut( &psz_html, "</s>" );
-                    break;
-                case 'f':
-                    HtmlPut( &psz_html, "</font>" );
-                    break;
-                case 'I':
-                    break;
-                }
-
-                *psz_last = '\0';
-            }
-            HtmlPut( &psz_html, psz_text_close );
-        }
-    }
     free( psz_tag );
 
-    return psz_html_start;
+    return psz_html;
 }
-



More information about the vlc-commits mailing list