[Easytag-mailing] [PATCH] mulitple stack corruption bugs in Scan_Generate_New_Tag_From_Mask

[Mark Ferry <gnomeza imap cc>]

Hi all

The following patch fixes three stack-corruption bugs in easytag-1.9.11
scan.c:Scan_Generate_New_Tag_From_Mask when the user enters stupidly
long scan masks

All three are code sections of the form:
len = strlen(char*)
strncpy(char[256],char*,len);

To reproduce:

Enter a fill-tag mask like:
  %a%t
Paste a long string of characters (more than 256) before %t
The strncopy in scan.c
498:          len = strlen(mask_seq) - strlen(tmp);
499:          strncpy(separator,mask_seq,len);
overflows separator and corrupts the stack.

The other two are similar and overflow "buf" and "string".

The solution now allocates 'separator', 'buf' and 'string' dynamically.
I experimented with keeping the allocations on the stack and clamping
the strncpy length to MIN(sizeof(buf)-1,len) but adding extra bounds
checking and variables to overcome the side-effects made for messy code.

This way seems cleaner, more memory efficient and allows the handling of 
much longer tags (albeit at a performance penalty for the extra strndups).

But perhaps I'm missing something and there was a good reason for 
declaring them as buf[256], separator[256] and string[1024]?

ciao
  Mark


--- easytag-1.99.11/src/scan.c.orig	2005-12-19 01:50:35.000000000 +0000
+++ easytag-1.99.11/src/scan.c	2005-12-19 00:22:33.000000000 +0000
@@ -360,9 +360,9 @@
     gchar *filename = NULL;
     gchar *filename_utf8;
     gchar *tmp;
-    gchar buf[256];
-    gchar separator[256];
-    gchar string[1024];
+    gchar *buf;
+    gchar *separator;
+    gchar *string;
     gint  len, i, loop=0;
     gchar **mask_splitted;
     gchar **file_splitted;
@@ -466,8 +466,7 @@
             if ( (len = strlen(mask_seq) - strlen(tmp)) > 0 )
             {
                 // Get this text in 'mask_seq'
-                strncpy(buf,mask_seq,len);
-                buf[len] = 0;
+                buf = g_strndup(mask_seq,len);
                 // We remove it in 'mask_seq'
                 mask_seq = mask_seq + len;
                 // Find the same text at the begining of 'file_seq' ?
@@ -478,6 +477,7 @@
                 {
                     g_print(_("Scan Error: can't find separator '%s'
within '%s'\n"),buf,file_seq_utf8);
                 }
+                g_free(buf);
             }

             // Remove the current code into 'mask_seq'
@@ -491,17 +491,16 @@
                 if ( (tmp=strchr(mask_seq,'%')) == NULL || strlen(tmp)
< 2 )
                 {
                     // No more code found
-                    strcpy(separator,mask_seq);
-                    separator[strlen(mask_seq)] = 0;
+                    len = strlen(mask_seq);
                 }else
                 {
                     len = strlen(mask_seq) - strlen(tmp);
-                    strncpy(separator,mask_seq,len);
-                    separator[len] = 0;
                 }

+                separator = g_strndup(mask_seq,len);
+
                 // Remove the current separator in 'mask_seq'
-                mask_seq = mask_seq + strlen(separator);
+                mask_seq = mask_seq + len;

                 // Try to find the separator in 'file_seq'
                 if ( (tmp=strstr(file_seq,separator)) == NULL )
@@ -512,14 +511,18 @@

                 // Get the string affected to the code (or the
corresponding entry field)
                 len = strlen(file_seq) - (tmp!=NULL?strlen(tmp):0);
+
+                string = g_malloc0(len + 1);
                 strncpy(string,file_seq,len);
                 string[len] = 0;

                 // Remove the current separator in 'file_seq'
-                file_seq = file_seq + strlen(string) + strlen(separator);
+                file_seq = file_seq + len + strlen(separator);

                 // We get the text affected to the code
-                mask_item->string = g_strdup(string);
+                mask_item->string = string;
+
+                g_free(separator);
             }else
             {
                 // We display the remaining text, affected to the code
(no more data in 'mask_seq')


		
___________________________________________________________ 
To help you stay safe and secure online, we've developed the all new Yahoo! Security Centre. http://uk.security.yahoo.com