Made reading/writing undo info a bit more robust.

4 files changed, 233 insertions, 163 deletions

diff --git a/src/ex_docmd.c b/src/ex_docmd.c
index 6d0ce2413e..c6d0b40ecb 100644
--- a/src/ex_docmd.c
+++ b/src/ex_docmd.c
@@ -8478,7 +8478,7 @@ ex_rundo(eap)
     char_u hash[UNDO_HASH_SIZE];
 
     u_compute_hash(hash);
-    u_read_undo(eap->arg, hash);
+    u_read_undo(eap->arg, hash, NULL);
 }
 #endif
 
diff --git a/src/fileio.c b/src/fileio.c
index f8375c65d8..101c804de8 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -2588,7 +2588,7 @@ failed:
 	char_u	hash[UNDO_HASH_SIZE];
 
 	sha256_finish(&sha_ctx, hash);
-	u_read_undo(NULL, hash);
+	u_read_undo(NULL, hash, fname);
     }
 #endif
 
diff --git a/src/proto/undo.pro b/src/proto/undo.pro
index cea7d49c51..baa98c4f87 100644
--- a/src/proto/undo.pro
+++ b/src/proto/undo.pro
@@ -9,7 +9,7 @@ int undo_allowed __ARGS((void));
 void u_compute_hash __ARGS((char_u *hash));
 char_u *u_get_undo_file_name __ARGS((char_u *buf_ffname, int reading));
 void u_write_undo __ARGS((char_u *name, int forceit, buf_T *buf, char_u *hash));
-void u_read_undo __ARGS((char_u *name, char_u *hash));
+void u_read_undo __ARGS((char_u *name, char_u *hash, char_u *orig_name));
 void u_undo __ARGS((int count));
 void u_redo __ARGS((int count));
 void undo_time __ARGS((long step, int sec, int absolute));
diff --git a/src/undo.c b/src/undo.c
index 34140f2a88..35b66c3db1 100644
--- a/src/undo.c
+++ b/src/undo.c
@@ -102,6 +102,9 @@ static void u_freeentry __ARGS((u_entry_T *, long));
 #ifdef FEAT_PERSISTENT_UNDO
 static void corruption_error __ARGS((char *msg, char_u *file_name));
 static void u_free_uhp __ARGS((u_header_T *uhp));
+static int serialize_header __ARGS((FILE *fp, buf_T *buf, char_u *hash));
+static int serialize_uhp __ARGS((FILE *fp, u_header_T *uhp));
+static u_header_T *unserialize_uhp __ARGS((FILE *fp, char_u *file_name));
 static int serialize_uep __ARGS((u_entry_T *uep, FILE *fp));
 static u_entry_T *unserialize_uep __ARGS((FILE *fp, int *error, char_u *file_name));
 static void serialize_pos __ARGS((pos_T pos, FILE *fp));
@@ -797,6 +800,173 @@ u_free_uhp(uhp)
     vim_free(uhp);
 }
 
+    static int
+serialize_header(fp, buf, hash)
+    FILE	*fp;
+    buf_T	*buf;
+    char_u	*hash;
+{
+    int len;
+
+    /* Start writing, first the magic marker and undo info version. */
+    if (fwrite(UF_START_MAGIC, (size_t)UF_START_MAGIC_LEN, (size_t)1, fp) != 1)
+	return FAIL;
+    put_bytes(fp, (long_u)UF_VERSION, 2);
+
+    /* Write a hash of the buffer text, so that we can verify it is still the
+     * same when reading the buffer text. */
+    if (fwrite(hash, (size_t)UNDO_HASH_SIZE, (size_t)1, fp) != 1)
+	return FAIL;
+
+    /* buffer-specific data */
+    put_bytes(fp, (long_u)buf->b_ml.ml_line_count, 4);
+    len = buf->b_u_line_ptr != NULL ? (int)STRLEN(buf->b_u_line_ptr) : 0;
+    put_bytes(fp, (long_u)len, 4);
+    if (len > 0 && fwrite(buf->b_u_line_ptr, (size_t)len, (size_t)1, fp) != 1)
+	return FAIL;
+    put_bytes(fp, (long_u)buf->b_u_line_lnum, 4);
+    put_bytes(fp, (long_u)buf->b_u_line_colnr, 4);
+
+    /* Undo structures header data */
+    put_header_ptr(fp, buf->b_u_oldhead);
+    put_header_ptr(fp, buf->b_u_newhead);
+    put_header_ptr(fp, buf->b_u_curhead);
+
+    put_bytes(fp, (long_u)buf->b_u_numhead, 4);
+    put_bytes(fp, (long_u)buf->b_u_seq_last, 4);
+    put_bytes(fp, (long_u)buf->b_u_seq_cur, 4);
+    put_time(fp, buf->b_u_seq_time);
+
+    return OK;
+}
+
+    static int
+serialize_uhp(fp, uhp)
+    FILE	*fp;
+    u_header_T	*uhp;
+{
+    int		i;
+    u_entry_T	*uep;
+
+    if (put_bytes(fp, (long_u)UF_HEADER_MAGIC, 2) == FAIL)
+	return FAIL;
+
+    put_header_ptr(fp, uhp->uh_next);
+    put_header_ptr(fp, uhp->uh_prev);
+    put_header_ptr(fp, uhp->uh_alt_next);
+    put_header_ptr(fp, uhp->uh_alt_prev);
+    put_bytes(fp, uhp->uh_seq, 4);
+    serialize_pos(uhp->uh_cursor, fp);
+#ifdef FEAT_VIRTUALEDIT
+    put_bytes(fp, (long_u)uhp->uh_cursor_vcol, 4);
+#else
+    put_bytes(fp, (long_u)0, 4);
+#endif
+    put_bytes(fp, (long_u)uhp->uh_flags, 2);
+    /* Assume NMARKS will stay the same. */
+    for (i = 0; i < NMARKS; ++i)
+	serialize_pos(uhp->uh_namedm[i], fp);
+#ifdef FEAT_VISUAL
+    serialize_visualinfo(&uhp->uh_visual, fp);
+#else
+    {
+	visualinfo_T info;
+
+	memset(&info, 0, sizeof(visualinfo_T));
+	serialize_visualinfo(&info, fp);
+    }
+#endif
+    put_time(fp, uhp->uh_time);
+
+    /* Write all the entries. */
+    for (uep = uhp->uh_entry; uep != NULL; uep = uep->ue_next)
+    {
+	put_bytes(fp, (long_u)UF_ENTRY_MAGIC, 2);
+	if (serialize_uep(uep, fp) == FAIL)
+	    return FAIL;
+    }
+    put_bytes(fp, (long_u)UF_ENTRY_END_MAGIC, 2);
+    return OK;
+}
+
+    static u_header_T *
+unserialize_uhp(fp, file_name)
+    FILE	*fp;
+    char_u	*file_name;
+{
+    u_header_T	*uhp;
+    int		i;
+    u_entry_T	*uep, *last_uep;
+    int		c;
+    int		error;
+
+    uhp = (u_header_T *)U_ALLOC_LINE(sizeof(u_header_T));
+    if (uhp == NULL)
+	return NULL;
+    vim_memset(uhp, 0, sizeof(u_header_T));
+#ifdef U_DEBUG
+    uhp->uh_magic = UH_MAGIC;
+#endif
+    /* We're not actually trying to store pointers here. We're just storing
+     * uh_seq numbers of the header pointed to, so we can swizzle them into
+     * pointers later - hence the type cast. */
+    uhp->uh_next = (u_header_T *)(long_u)get4c(fp);
+    uhp->uh_prev = (u_header_T *)(long_u)get4c(fp);
+    uhp->uh_alt_next = (u_header_T *)(long_u)get4c(fp);
+    uhp->uh_alt_prev = (u_header_T *)(long_u)get4c(fp);
+    uhp->uh_seq = get4c(fp);
+    if (uhp->uh_seq <= 0)
+    {
+	corruption_error("uh_seq", file_name);
+	vim_free(uhp);
+	return NULL;
+    }
+    unserialize_pos(&uhp->uh_cursor, fp);
+#ifdef FEAT_VIRTUALEDIT
+    uhp->uh_cursor_vcol = get4c(fp);
+#else
+    (void)get4c(fp);
+#endif
+    uhp->uh_flags = get2c(fp);
+    for (i = 0; i < NMARKS; ++i)
+	unserialize_pos(&uhp->uh_namedm[i], fp);
+#ifdef FEAT_VISUAL
+    unserialize_visualinfo(&uhp->uh_visual, fp);
+#else
+    {
+	visualinfo_T info;
+	unserialize_visualinfo(&info, fp);
+    }
+#endif
+    uhp->uh_time = get8ctime(fp);
+
+    /* Unserialize the uep list. */
+    last_uep = NULL;
+    while ((c = get2c(fp)) == UF_ENTRY_MAGIC)
+    {
+	error = FALSE;
+	uep = unserialize_uep(fp, &error, file_name);
+	if (last_uep == NULL)
+	    uhp->uh_entry = uep;
+	else
+	    last_uep->ue_next = uep;
+	last_uep = uep;
+	if (uep == NULL || error)
+	{
+	    u_free_uhp(uhp);
+	    return NULL;
+	}
+    }
+    if (c != UF_ENTRY_END_MAGIC)
+    {
+	corruption_error("entry end", file_name);
+	u_free_uhp(uhp);
+	return NULL;
+    }
+
+    return uhp;
+}
+
 /*
  * Serialize "uep" to "fp".
  */
@@ -830,7 +1000,6 @@ unserialize_uep(fp, error, file_name)
     char_u	*file_name;
 {
     int		i;
-    int		j;
     u_entry_T	*uep;
     char_u	**array;
     char_u	*line;
@@ -865,7 +1034,7 @@ unserialize_uep(fp, error, file_name)
     {
 	line_len = get4c(fp);
 	if (line_len >= 0)
-	    line = (char_u *)U_ALLOC_LINE(line_len + 1);
+	    line = read_string(fp, line_len);
 	else
 	{
 	    line = NULL;
@@ -876,9 +1045,6 @@ unserialize_uep(fp, error, file_name)
 	    *error = TRUE;
 	    return uep;
 	}
-	for (j = 0; j < line_len; j++)
-	    line[j] = getc(fp);
-	line[j] = NUL;
 	array[i] = line;
     }
     return uep;
@@ -910,9 +1076,15 @@ unserialize_pos(pos, fp)
     FILE  *fp;
 {
     pos->lnum = get4c(fp);
+    if (pos->lnum < 0)
+	pos->lnum = 0;
     pos->col = get4c(fp);
+    if (pos->col < 0)
+	pos->col = 0;
 #ifdef FEAT_VIRTUALEDIT
     pos->coladd = get4c(fp);
+    if (pos->coladd < 0)
+	pos->coladd = 0;
 #else
     (void)get4c(fp);
 #endif
@@ -975,9 +1147,8 @@ u_write_undo(name, forceit, buf, hash)
     char_u	*hash;
 {
     u_header_T	*uhp;
-    u_entry_T	*uep;
     char_u	*file_name;
-    int		str_len, i, mark;
+    int		mark;
 #ifdef U_DEBUG
     int		headers_written = 0;
 #endif
@@ -1069,7 +1240,8 @@ u_write_undo(name, forceit, buf, hash)
 		    {
 			if (name == NULL)
 			    verbose_enter();
-			smsg((char_u *)_("Will not overwrite, this is not an undo file: %s"),
+			smsg((char_u *)
+			_("Will not overwrite, this is not an undo file: %s"),
 								   file_name);
 			if (name == NULL)
 			    verbose_leave();
@@ -1083,7 +1255,7 @@ u_write_undo(name, forceit, buf, hash)
 
     /* If there is no undo information at all, quit here after deleting any
      * existing undo file. */
-    if (buf->b_u_numhead == 0)
+    if (buf->b_u_numhead == 0 && buf->b_u_line_ptr == NULL)
     {
 	if (p_verbose > 0)
 	    verb_msg((char_u *)_("Skipping undo file write, noting to undo"));
@@ -1141,37 +1313,12 @@ u_write_undo(name, forceit, buf, hash)
     /* Undo must be synced. */
     u_sync(TRUE);
 
-    /* Start writing, first the undo file header. */
-    if (fwrite(UF_START_MAGIC, (size_t)UF_START_MAGIC_LEN, (size_t)1, fp) != 1)
-	goto write_error;
-    put_bytes(fp, (long_u)UF_VERSION, 2);
-
-    /* Write a hash of the buffer text, so that we can verify it is still the
-     * same when reading the buffer text. */
-    if (fwrite(hash, (size_t)UNDO_HASH_SIZE, (size_t)1, fp) != 1)
-	goto write_error;
-    put_bytes(fp, (long_u)buf->b_ml.ml_line_count, 4);
-
-    /* Begin undo data for U */
-    str_len = buf->b_u_line_ptr != NULL ? (int)STRLEN(buf->b_u_line_ptr) : 0;
-    put_bytes(fp, (long_u)str_len, 4);
-    if (str_len > 0 && fwrite(buf->b_u_line_ptr, (size_t)str_len,
-							  (size_t)1, fp) != 1)
+    /*
+     * Write the header.
+     */
+    if (serialize_header(fp, buf, hash) == FAIL)
 	goto write_error;
 
-    put_bytes(fp, (long_u)buf->b_u_line_lnum, 4);
-    put_bytes(fp, (long_u)buf->b_u_line_colnr, 4);
-
-    /* Begin general undo data */
-    put_header_ptr(fp, buf->b_u_oldhead);
-    put_header_ptr(fp, buf->b_u_newhead);
-    put_header_ptr(fp, buf->b_u_curhead);
-
-    put_bytes(fp, (long_u)buf->b_u_numhead, 4);
-    put_bytes(fp, (long_u)buf->b_u_seq_last, 4);
-    put_bytes(fp, (long_u)buf->b_u_seq_cur, 4);
-    put_time(fp, buf->b_u_seq_time);
-
     /*
      * Iteratively serialize UHPs and their UEPs from the top down.
      */
@@ -1186,48 +1333,11 @@ u_write_undo(name, forceit, buf, hash)
 #ifdef U_DEBUG
 	    ++headers_written;
 #endif
-
-	    if (put_bytes(fp, (long_u)UF_HEADER_MAGIC, 2) == FAIL)
+	    if (serialize_uhp(fp, uhp) == FAIL)
 		goto write_error;
-
-	    put_header_ptr(fp, uhp->uh_next);
-	    put_header_ptr(fp, uhp->uh_prev);
-	    put_header_ptr(fp, uhp->uh_alt_next);
-	    put_header_ptr(fp, uhp->uh_alt_prev);
-            put_bytes(fp, uhp->uh_seq, 4);
-            serialize_pos(uhp->uh_cursor, fp);
-#ifdef FEAT_VIRTUALEDIT
-            put_bytes(fp, (long_u)uhp->uh_cursor_vcol, 4);
-#else
-            put_bytes(fp, (long_u)0, 4);
-#endif
-            put_bytes(fp, (long_u)uhp->uh_flags, 2);
-            /* Assume NMARKS will stay the same. */
-            for (i = 0; i < NMARKS; ++i)
-                serialize_pos(uhp->uh_namedm[i], fp);
-#ifdef FEAT_VISUAL
-            serialize_visualinfo(&uhp->uh_visual, fp);
-#else
-	    {
-		visualinfo_T info;
-
-		memset(&info, 0, sizeof(visualinfo_T));
-		serialize_visualinfo(&info, fp);
-	    }
-#endif
-            put_time(fp, uhp->uh_time);
-
-	    /* Write all the entries. */
-	    for (uep = uhp->uh_entry; uep != NULL; uep = uep->ue_next)
-	    {
-		put_bytes(fp, (long_u)UF_ENTRY_MAGIC, 2);
-                if (serialize_uep(uep, fp) == FAIL)
-		    goto write_error;
-	    }
-	    put_bytes(fp, (long_u)UF_ENTRY_END_MAGIC, 2);
         }
 
-        /* Now walk through the tree - algorithm from undo_time */
+        /* Now walk through the tree - algorithm from undo_time(). */
         if (uhp->uh_prev != NULL && uhp->uh_prev->uh_walk != mark)
             uhp = uhp->uh_prev;
         else if (uhp->uh_alt_next != NULL && uhp->uh_alt_next->uh_walk != mark)
@@ -1255,6 +1365,8 @@ write_error:
         EMSG2(_("E829: write error in undo file: %s"), file_name);
 
 #if defined(MACOS_CLASSIC) || defined(WIN3264)
+    /* Copy file attributes; for systems where this can only be done after
+     * closing the file. */
     if (buf->b_ffname != NULL)
 	(void)mch_copy_file_attribute(buf->b_ffname, file_name);
 #endif
@@ -1282,9 +1394,10 @@ theend:
  * "hash[UNDO_HASH_SIZE]" must be the hash value of the buffer text.
  */
     void
-u_read_undo(name, hash)
+u_read_undo(name, hash, orig_name)
     char_u *name;
     char_u *hash;
+    char_u *orig_name;
 {
     char_u	*file_name;
     FILE	*fp;
@@ -1301,7 +1414,6 @@ u_read_undo(name, hash)
     time_t	seq_time;
     int		i, j;
     int		c;
-    u_entry_T	*uep, *last_uep;
     u_header_T	*uhp;
     u_header_T	**uhp_table = NULL;
     char_u	read_hash[UNDO_HASH_SIZE];
@@ -1309,12 +1421,34 @@ u_read_undo(name, hash)
 #ifdef U_DEBUG
     int		*uhp_table_used;
 #endif
+#ifdef UNIX
+    struct stat	st_orig;
+    struct stat	st_undo;
+#endif
 
     if (name == NULL)
     {
         file_name = u_get_undo_file_name(curbuf->b_ffname, TRUE);
 	if (file_name == NULL)
 	    return;
+
+#ifdef UNIX
+	/* For safety we only read an undo file if the owner is equal to the
+	 * owner of the text file. */
+	if (mch_stat((char *)orig_name, &st_orig) >= 0
+		&& mch_stat((char *)file_name, &st_undo) >= 0
+		&& st_orig.st_uid != st_undo.st_uid)
+	{
+	    if (p_verbose > 0)
+	    {
+		verbose_enter();
+		smsg((char_u *)_("Not reading undo file, owner differs: %s"),
+								   file_name);
+		verbose_leave();
+	    }
+	    return;
+	}
+#endif
     }
     else
         file_name = name;
@@ -1325,6 +1459,7 @@ u_read_undo(name, hash)
 	smsg((char_u *)_("Reading undo file: %s"), file_name);
 	verbose_leave();
     }
+
     fp = mch_fopen((char *)file_name, "r");
     if (fp == NULL)
     {
@@ -1362,27 +1497,27 @@ u_read_undo(name, hash)
         {
 	    if (name == NULL)
 		verbose_enter();
-            give_warning((char_u *)_("File contents changed, cannot use undo info"), TRUE);
+            give_warning((char_u *)
+		      _("File contents changed, cannot use undo info"), TRUE);
 	    if (name == NULL)
 		verbose_leave();
         }
         goto error;
     }
 
-    /* Begin undo data for U */
+    /* Read undo data for "U" command. */
     str_len = get4c(fp);
     if (str_len < 0)
         goto error;
-    else if (str_len > 0)
-    {
-        if ((line_ptr = U_ALLOC_LINE(str_len + 1)) == NULL)
-            goto error;
-        for (i = 0; i < str_len; i++)
-            line_ptr[i] = (char_u)getc(fp);
-        line_ptr[i] = NUL;
-    }
+    if (str_len > 0)
+	line_ptr = read_string(fp, str_len);
     line_lnum = (linenr_T)get4c(fp);
     line_colnr = (colnr_T)get4c(fp);
+    if (line_lnum < 0 || line_colnr < 0)
+    {
+	corruption_error("line lnum/col", file_name);
+	goto error;
+    }
 
     /* Begin general undo data */
     old_header_seq = get4c(fp);
@@ -1409,76 +1544,13 @@ u_read_undo(name, hash)
     {
 	if (num_read_uhps >= num_head)
 	{
-	    corruption_error("num_head", file_name);
+	    corruption_error("num_head too small", file_name);
 	    goto error;
 	}
 
-        uhp = (u_header_T *)U_ALLOC_LINE(sizeof(u_header_T));
-        if (uhp == NULL)
-            goto error;
-        vim_memset(uhp, 0, sizeof(u_header_T));
-#ifdef U_DEBUG
-	uhp->uh_magic = UH_MAGIC;
-#endif
-        /* We're not actually trying to store pointers here. We're just storing
-         * IDs so we can swizzle them into pointers later - hence the type
-	 * cast. */
-        uhp->uh_next = (u_header_T *)(long_u)get4c(fp);
-        uhp->uh_prev = (u_header_T *)(long_u)get4c(fp);
-        uhp->uh_alt_next = (u_header_T *)(long_u)get4c(fp);
-        uhp->uh_alt_prev = (u_header_T *)(long_u)get4c(fp);
-        uhp->uh_seq = get4c(fp);
-        if (uhp->uh_seq <= 0)
-        {
-	    corruption_error("uh_seq", file_name);
-            vim_free(uhp);
-            goto error;
-        }
-        uhp->uh_walk = 0;
-        unserialize_pos(&uhp->uh_cursor, fp);
-#ifdef FEAT_VIRTUALEDIT
-        uhp->uh_cursor_vcol = get4c(fp);
-#else
-        (void)get4c(fp);
-#endif
-        uhp->uh_flags = get2c(fp);
-        for (i = 0; i < NMARKS; ++i)
-            unserialize_pos(&uhp->uh_namedm[i], fp);
-#ifdef FEAT_VISUAL
-        unserialize_visualinfo(&uhp->uh_visual, fp);
-#else
-	{
-	    visualinfo_T info;
-	    unserialize_visualinfo(&info, fp);
-	}
-#endif
-        uhp->uh_time = get8ctime(fp);
-
-        /* Unserialize the uep list. */
-        last_uep = NULL;
-        while ((c = get2c(fp)) == UF_ENTRY_MAGIC)
-        {
-	    int error = FALSE;
-
-	    uep = unserialize_uep(fp, &error, file_name);
-            if (last_uep == NULL)
-                uhp->uh_entry = uep;
-	    else
-                last_uep->ue_next = uep;
-            last_uep = uep;
-	    if (uep == NULL || error)
-	    {
-		u_free_uhp(uhp);
-                goto error;
-	    }
-        }
-	if (c != UF_ENTRY_END_MAGIC)
-        {
-	    corruption_error("entry end", file_name);
-            u_free_uhp(uhp);
-            goto error;
-        }
-
+	uhp = unserialize_uhp(fp, file_name);
+	if (uhp == NULL)
+	    goto error;
 	uhp_table[num_read_uhps++] = uhp;
     }
 
@@ -1487,7 +1559,6 @@ u_read_undo(name, hash)
 	corruption_error("num_head", file_name);
 	goto error;
     }
-
     if (c != UF_HEADER_END_MAGIC)
     {
 	corruption_error("end marker", file_name);
@@ -1502,8 +1573,8 @@ u_read_undo(name, hash)
 # define SET_FLAG(j)
 #endif
 
-    /* We have put all of the uhps into a table. Now we iterate through the
-     * table and swizzle each sequence number we've stored in uh_* into a
+    /* We have put all of the headers into a table. Now we iterate through the
+     * table and swizzle each sequence number we have stored in uh_* into a
      * pointer corresponding to the header with that sequence number. */
     for (i = 0; i < num_head; i++)
     {
@@ -1519,7 +1590,6 @@ u_read_undo(name, hash)
 		corruption_error("duplicate uh_seq", file_name);
                 goto error;
 	    }
-
             if (uhp_table[j]->uh_seq == (long)uhp->uh_next)
 	    {
                 uhp->uh_next = uhp_table[j];