changeset 2223:81b83a19e127 vim73

More strict checks for the undo file.
author Bram Moolenaar <bram@vim.org>
date Wed, 26 May 2010 21:21:00 +0200
parents 271a5907f944
children a0cce15dd2a9
files runtime/doc/tags runtime/doc/todo.txt runtime/doc/undo.txt src/undo.c
diffstat 4 files changed, 74 insertions(+), 34 deletions(-) [+]
line wrap: on
line diff
--- a/runtime/doc/tags
+++ b/runtime/doc/tags
@@ -4166,7 +4166,17 @@ E819	editing.txt	/*E819*
 E82	message.txt	/*E82*
 E820	editing.txt	/*E820*
 E821	options.txt	/*E821*
+E822	undo.txt	/*E822*
+E823	undo.txt	/*E823*
+E824	undo.txt	/*E824*
+E825	undo.txt	/*E825*
+E826	undo.txt	/*E826*
+E827	undo.txt	/*E827*
+E828	undo.txt	/*E828*
+E829	undo.txt	/*E829*
 E83	message.txt	/*E83*
+E830	undo.txt	/*E830*
+E831	undo.txt	/*E831*
 E84	windows.txt	/*E84*
 E85	options.txt	/*E85*
 E86	windows.txt	/*E86*
--- a/runtime/doc/todo.txt
+++ b/runtime/doc/todo.txt
@@ -33,6 +33,9 @@ be worked on, but only if you sponsor Vi
 When Vim crashes it may run out of stack while executing autocommands.  Patch
 to not run autocommands when leaving Vim? (James Vega, 2010 May 23)
 
+Invalid memory access when deleting funcref variable.  Patch by Lech Lorens,
+2010 May 25.
+
 Cursor positioning wrong with 0x200e character. (John Becket, 2010 May 6)
 
 E315 when trying to change a file in FileChangedRO autocommand event.
@@ -1094,8 +1097,12 @@ Vim 7.3:
   Wait until window is gone with EnumWindows (see os_win32.c).
 Patches to include:
 - Persistent undo bugs / fixes:
+    - Patch not to allocate extra byte in U_ALLOC_LINE() (Dominique, 2010 May
+      25)
+    - Remove the old code when U_USE_MALLOC is not defined?
+    - When there is no undo info (undolevels negative), delete the undo file.
+    - Need to check all values for evil manipulation.
     - Add undofile(name): get undo file name for buffer "name".
-    - When there is no undo info (undolevels negative), delete the undo file.
 - Extend test62 for gettabvar() and settabvar(). (Yegappan Lakshmanan, 2010
   May 23)
 - Also crypt the undo file.
--- a/runtime/doc/undo.txt
+++ b/runtime/doc/undo.txt
@@ -267,8 +267,8 @@ Reading an existing undo file may fail f
 	The file text differs from when the undo file was written.  This means
 	the undo file cannot be used, it would corrupt the text.  This also
 	happens when 'encoding' differs from when the undo file was written.
-*E825* *E826*	The undo file does not contain valid contents and cannot be
-	used.
+*E825* *E826* *E831*
+	The undo file does not contain valid contents and cannot be used.
 *E827*	The magic number at the end of the file was not found.  This usually
 	means the file was truncated.
 
--- a/src/undo.c
+++ b/src/undo.c
@@ -107,6 +107,7 @@ static void u_freeentry __ARGS((u_entry_
 static void unserialize_pos __ARGS((pos_T *pos, FILE *fp));
 static void unserialize_visualinfo __ARGS((visualinfo_T *info, FILE *fp));
 static char_u *u_get_undo_file_name __ARGS((char_u *, int reading));
+static void u_free_uhp __ARGS((u_header_T *uhp));
 static int serialize_uep __ARGS((u_entry_T *uep, FILE *fp));
 static void serialize_pos __ARGS((pos_T pos, FILE *fp));
 static void serialize_visualinfo __ARGS((visualinfo_T info, FILE *fp));
@@ -835,10 +836,9 @@ u_read_undo(name, hash)
     time_t	seq_time;
     int		i, j;
     int		c;
-    short	found_first_uep = 0;
     char_u	**array;
     char_u	*line;
-    u_entry_T	*uep, *last_uep, *nuep;
+    u_entry_T	*uep, *last_uep;
     u_header_T	*uhp;
     u_header_T	**uhp_table = NULL;
     char_u	read_hash[UNDO_HASH_SIZE];
@@ -914,6 +914,9 @@ u_read_undo(name, hash)
     seq_cur = get4c(fp);
     seq_time = get4c(fp);
 
+    if (num_head < 0)
+	num_head = 0;
+
     /* uhp_table will store the freshly created undo headers we allocate
      * until we insert them into curbuf. The table remains sorted by the
      * sequence numbers of the headers. */
@@ -925,7 +928,13 @@ u_read_undo(name, hash)
     c = get2c(fp);
     while (c == UF_HEADER_MAGIC)
     {
-        found_first_uep = 0;
+	if (num_read_uhps >= num_head)
+	{
+	    EMSG2(_("E831 Undo file corruption: num_head: %s"), file_name);
+	    u_free_uhp(uhp);
+	    goto error;
+	}
+
         uhp = (u_header_T *)U_ALLOC_LINE((unsigned)sizeof(u_header_T));
         if (uhp == NULL)
             goto error;
@@ -973,8 +982,17 @@ u_read_undo(name, hash)
         {
             uep = (u_entry_T *)U_ALLOC_LINE((unsigned)sizeof(u_entry_T));
             if (uep == NULL)
+	    {
+		u_free_uhp(uhp);
                 goto error;
+	    }
             vim_memset(uep, 0, sizeof(u_entry_T));
+            if (last_uep == NULL)
+                uhp->uh_entry = uep;
+	    else
+                last_uep->ue_next = uep;
+            last_uep = uep;
+
             uep->ue_top = get4c(fp);
             uep->ue_bot = get4c(fp);
             uep->ue_lcount = get4c(fp);
@@ -982,37 +1000,35 @@ u_read_undo(name, hash)
             uep->ue_next = NULL;
             array = (char_u **)U_ALLOC_LINE(
 				 (unsigned)(sizeof(char_u *) * uep->ue_size));
+	    if (array == NULL)
+	    {
+		u_free_uhp(uhp);
+		goto error;
+	    }
+            vim_memset(array, 0, sizeof(char_u *) * uep->ue_size);
+            uep->ue_array = array;
+
             for (i = 0; i < uep->ue_size; i++)
             {
                 line_len = get4c(fp);
                 /* U_ALLOC_LINE provides an extra byte for the NUL terminator.*/
                 line = (char_u *)U_ALLOC_LINE(
-				      (unsigned) (sizeof(char_u) * line_len));
+				       (unsigned)(sizeof(char_u) * line_len));
                 if (line == NULL)
+		{
+		    u_free_uhp(uhp);
                     goto error;
+		}
                 for (j = 0; j < line_len; j++)
-                {
                     line[j] = getc(fp);
-                }
                 line[j] = '\0';
                 array[i] = line;
             }
-            uep->ue_array = array;
-            if (found_first_uep == 0)
-            {
-                uhp->uh_entry = uep;
-                found_first_uep = 1;
-            }
-            else
-            {
-                last_uep->ue_next = uep;
-            }
-            last_uep = uep;
         }
 
         /* Insertion sort the uhp into the table by its uh_seq. This is
          * required because, while the number of uhps is limited to
-         * num_heads, and the uh_seq order is monotonic with respect to
+         * num_head, and the uh_seq order is monotonic with respect to
          * creation time, the starting uh_seq can be > 0 if any undolevel
          * culling was done at undofile write time, and there can be uh_seq
          * gaps in the uhps.
@@ -1028,7 +1044,7 @@ u_read_undo(name, hash)
                 if (num_read_uhps - i - 1 > 0)
                 {
                     memmove(uhp_table + i + 2, uhp_table + i + 1,
-                            (num_read_uhps - i - 1) * sizeof(u_header_T *));
+			      (num_read_uhps - i - 1) * sizeof(u_header_T *));
                 }
                 uhp_table[i + 1] = uhp;
                 break;
@@ -1037,6 +1053,7 @@ u_read_undo(name, hash)
             {
                 EMSG2(_("E826 Undo file corruption: duplicate uh_seq: %s"),
 								   file_name);
+		u_free_uhp(uhp);
                 goto error;
             }
         }
@@ -1105,19 +1122,8 @@ error:
     if (uhp_table != NULL)
     {
         for (i = 0; i < num_head; i++)
-        {
             if (uhp_table[i] != NULL)
-            {
-                uep = uhp_table[i]->uh_entry;
-                while (uep != NULL)
-                {
-                    nuep = uep->ue_next;
-                    u_freeentry(uep, uep->ue_size);
-                    uep = nuep;
-                }
-                U_FREE_LINE(uhp_table[i]);
-            }
-        }
+		u_free_uhp(uhp_table[i]);
         U_FREE_LINE(uhp_table);
     }
 
@@ -1129,6 +1135,23 @@ theend:
     return;
 }
 
+    static void
+u_free_uhp(uhp)
+    u_header_T	*uhp;
+{
+    u_entry_T	*nuep;
+    u_entry_T	*uep;
+
+    uep = uhp->uh_entry;
+    while (uep != NULL)
+    {
+	nuep = uep->ue_next;
+	u_freeentry(uep, uep->ue_size);
+	uep = nuep;
+    }
+    U_FREE_LINE(uhp);
+}
+
 /*
  * Serialize "uep" to "fp".
  */