Changeset 1154

Timestamp:

09/05/08 22:37:19 (3 months ago)

Author:

stevenhl

Message:

Correct FreeCnrItemData? pszDisplayName pointer overlap check

Files:

• trunk/dll/filldir.c (modified) (10 diffs)

trunk/dll/filldir.c

@@ -47 +47 @@
   02 Aug 08 GKY Remove redundant strcpys from inner loop
   23 Aug 08 GKY Free pszDisplayName when appropriate
-  01 Sep 08 GKY Updated FreeCnrItemData toprevent trap in strrchr if pci->pszFileName is NULL.
+  01 Sep 08 GKY Updated FreeCnrItemData to prevent trap in strrchr if pci->pszFileName is NULL.
+  05 Sep 08 SHL Correct FreeCnrItemData pszDisplayName pointer overlap check
 
 ***********************************************************************/
@@ -53 +54 @@
 #include <stdlib.h>
 #include <string.h>
+#include <malloc.h>                     // _msize _heapchk
 #include <ctype.h>
-
-#if 0 // fixme to disable or to be configurable
-#include <malloc.h>                     // _heapchk
-#endif
 
 #define INCL_DOS
@@ -270 +268 @@
 #   ifdef FORTIFY
     {
-      unsigned tid = GetTidForWindow(hwndCnr);
-      // char buf[256];
-      if (tid == 1)
+      if (dcd->type != TREE_FRAME)
         Fortify_ChangeScope(pci->pszFileName, -1);
-      else
+      else {
         Fortify_SetOwner(pci->pszFileName, 1);
-      // sprintf(buf, "Owner forced to %u", GetTidForWindow(hwndCnr));
-      // Fortify_LabelPointer(pci->pszFmtFileSize, buf);
+        Fortify_SetScope(pci->pszFileName, 2);
+      }
     }
 #   endif
@@ -374 +370 @@
             pci->pszLongName = xstrdup(value + (sizeof(USHORT) * 2), pszSrcFile, __LINE__);
 #           ifdef FORTIFY
-            {
-              unsigned tid = GetTidForWindow(hwndCnr);
-              // char buf[256];
-              if (tid == 1)
-                Fortify_ChangeScope(pci->pszLongName, -1);
-              else
-                Fortify_SetOwner(pci->pszLongName, 1);
-              // sprintf(buf, "Owner forced to %u", GetTidForWindow(hwndCnr));
-              // Fortify_LabelPointer(pci->pszFmtFileSize, buf);
-            }
+            {
+              unsigned tid = GetTidForWindow(hwndCnr);
+              if (tid == 1)
+                Fortify_ChangeScope(pci->pszLongName, -1);
+              else
+                Fortify_SetOwner(pci->pszLongName, 1);
+            }
 #           endif
           }
@@ -465 +458 @@
     {
       unsigned tid = GetTidForWindow(hwndCnr);
-      // char buf[256];
       if (tid == 1)
         Fortify_ChangeScope(pci->pszFmtFileSize, -1);
       else
         Fortify_SetOwner(pci->pszFmtFileSize, 1);
-      // sprintf(buf, "Owner forced to %u", GetTidForWindow(hwndCnr));
-      // Fortify_LabelPointer(pci->pszFmtFileSize, buf);
     }
 #   endif
@@ -529 +519 @@
 } // FillInRecordFromFFB
 
-ULONGLONG FillInRecordFromFSA(HWND hwndCnr, PCNRITEM pci,
+ULONGLONG FillInRecordFromFSA(HWND hwndCnr,
+                              PCNRITEM pci,
                               const PSZ pszFileName,
                               const PFILESTATUS4L pfsa4,
-                              const BOOL partial, DIRCNRDATA * dcd)     // Optional
+                              const BOOL partial,
+                              DIRCNRDATA *dcd)  // Optional
 {
   HPOINTER hptr;
@@ -691 +683 @@
   //comma format the file size for large file support
   {
-  CHAR szBuf[30];
+    CHAR szBuf[30];
     CommaFmtULL(szBuf, sizeof(szBuf), pfsa4->cbFile, ' ');
     pci->pszFmtFileSize = xstrdup(szBuf, pszSrcFile, __LINE__);
+#   ifdef FORTIFY
+    {
+      if (dcd && dcd->type == TREE_FRAME) {
+        // Will be freed in TreeCnrWndProc WM_DESTROY
+        // Fortify_SetOwner(pci->pszFmtFileSize, 1);
+        Fortify_SetScope(pci->pszFmtFileSize, 2);
+      }
+    }
+#   endif
   }
   pci->date.day = pfsa4->fdateLastWrite.day;
@@ -1289 +1290 @@
           pci->rc.hptrIcon = hptrDunno;
           pci->pszFileName = xstrdup(szDrive, pszSrcFile, __LINE__);
-          //strcpy(pci->pszFileName, szDrive);
+          // strcpy(pci->pszFileName, szDrive); // 22 Jul 08 SHL No need to do this twice
+#         ifdef FORTIFY
+          // Will be freed by TreeCnrWndProc WM_DESTROY
+          Fortify_SetScope(pci->pszFileName, 2);
+#         endif
           pci->pszDisplayName = pci->pszFileName;
           pci->rc.pszIcon = pci->pszFileName;
@@ -1390 +1395 @@
         char *p, *pp;
 
-        p = pszTreeEnvVarList;
+        p = pszTreeEnvVarList;
         while (*p == ';')
           p++;
@@ -1642 +1647 @@
     free(psz);
   }
+
+  // Check double free
   if (!pci->pszFileName)
-    DbgMsg(pszSrcFile, __LINE__, "FreeCnrItemData pci->pszFileName is NULL");
-  if (pci->pszFileName &&
+    DbgMsg(pszSrcFile, __LINE__, "FreeCnrItemData pci->pszFileName already NULL");
+
+  // Bypass free if pszDisplayName points into pszFileName buffer
+  // 05 Sep 08 SHL Correct pointer overlap compare logic
+  if (pci->pszDisplayName != pci->pszFileName &&
       pci->pszDisplayName &&
-      pci->pszDisplayName != NullStr &&
-      pci->pszDisplayName != pci->pszFileName &&
-      pci->pszDisplayName != strrchr(pci->pszFileName, '\\') &&
-      pci->pszDisplayName != strrchr(pci->pszFileName, ':') &&
-      pci->pszDisplayName != strrchr(pci->pszFileName, ':') + 1 &&
-      pci->pszDisplayName != strrchr(pci->pszFileName, '\\') + 1) {
-    psz = pci->pszDisplayName;
-    //pci->pszDisplayName = NullStr;
-    pci->pszDisplayName = NULL;         // for debug
-    free(psz);
-}
+      pci->pszDisplayName != NullStr) {
+    if (!pci->pszFileName ||
+        pci->pszDisplayName < pci->pszFileName ||
+        pci->pszDisplayName >= pci->pszFileName + _msize(pci->pszFileName))
+    {
+      psz = pci->pszDisplayName;
+      // pci->pszDisplayName = NullStr;
+      pci->pszDisplayName = NULL;               // for debug
+      free(psz);
+    }
+  }
 
   if (pci->pszFileName && pci->pszFileName != NullStr) {