r19638 - gnucash/trunk/src/backend/xml - Bug #593479: Ensure not to accidentally delete our main account file.

Christian Stimming cstim at code.gnucash.org
Tue Oct 5 14:07:38 EDT 2010


Author: cstim
Date: 2010-10-05 14:07:38 -0400 (Tue, 05 Oct 2010)
New Revision: 19638
Trac: http://svn.gnucash.org/trac/changeset/19638

Modified:
   gnucash/trunk/src/backend/xml/gnc-backend-xml.c
Log:
Bug #593479: Ensure not to accidentally delete our main account file.

Original patch by Tim Retout who writes:

strptime is passed (name + pathlen + 1) as the string to search.  However, when
looking at the main account file, strlen(name) == pathlen, so strptime is
looking at the point just past the end of name.

Sometimes this will be parseable by strptime, and this leads to the account
file being unlinked.

Modified: gnucash/trunk/src/backend/xml/gnc-backend-xml.c
===================================================================
--- gnucash/trunk/src/backend/xml/gnc-backend-xml.c	2010-10-05 18:03:51 UTC (rev 19637)
+++ gnucash/trunk/src/backend/xml/gnc-backend-xml.c	2010-10-05 18:07:38 UTC (rev 19638)
@@ -834,16 +834,24 @@
             continue;
 
         name = g_build_filename(be->dirname, dent, (gchar*)NULL);
-        len = strlen(name) - 4;
+        len = strlen(name);
 
         /* Never remove the current data file itself */
         if (g_strcmp0(name, be->fullpath) == 0)
             continue;
 
-        /* Is this file associated with the current data file */
-        if (strncmp(name, be->fullpath, pathlen) == 0)
+        /* Is this file associated with the current data file? 
+         * Additionally, the invariants for the pointer arithmetic
+         * must hold: String length long enough to contain the suffix,
+         * and string length large enough so that strptime below will
+         * not be passed a pointer outside of our string. (Otherwise
+         * the result of strptime might be parseable and the main data
+         * file is deleted, #593479) */
+        if ((strncmp(name, be->fullpath, pathlen) == 0)
+            && (len >= 4)
+            && (len > pathlen))
         {
-            if (safe_strcmp(name + len, ".LNK") == 0)
+            if (safe_strcmp(name + len - 4, ".LNK") == 0)
             {
                 /* Is a lock file. Skip the active lock file */
                 if ((safe_strcmp(name, be->linkfile) != 0) &&



More information about the gnucash-changes mailing list