nsIFile unique file creation is racy and insecure

bug 43314. a=brendan@mozilla.org
written by Robert O'Callahan <roc+moz@cs.cmu.edu>

xpcom/io/nsIFile.idl

@@ -77,7 +77,7 @@ interface nsIFile : nsISupports
      *       error (NS_ERROR_FILE_UNKNOWN_TYPE).
      *
      *   @param permissions
-     *       This unix style octal permissions.  This may
+     *       The unix style octal permissions.  This may
      *       be ignored on systems that do not need to do
      *       permissions.
      */
@@ -230,10 +230,32 @@ interface nsIFile : nsISupports
      */
     boolean isSpecial();
 
-    /**
-     * .
+   /**
+     *  createUnique
+     *  
+     *  This function will create a new file or directory in the
+     *  file system. Any nodes that have not been created or
+     *  resolved, will be.  If this file already exists, we try
+     *  variations on the leaf name "suggestedName" until we find
+     *  one that did not already exist.
+     *
+     *  If the search for nonexistent files takes too long
+     *  (thousands of the variants already exist), we give up and
+     *  return NS_ERROR_FILE_TOO_BIG.
+     *
+     *   @param type
+     *       This specifies the type of file system object
+     *       to be made.  The only two types at this time
+     *       are file and directory which are defined above.
+     *       If the type is unrecongnized, we will return an
+     *       error (NS_ERROR_FILE_UNKNOWN_TYPE).
+     *
+     *   @param permissions
+     *       The unix style octal permissions.  This may
+     *       be ignored on systems that do not need to do
+     *       permissions.
      */
-    void makeUnique(in string suggestedName);
+    void createUnique(in string suggestedName, in unsigned long type, in unsigned long permissions);
     
 
     /**

xpcom/io/nsLocalFileCommon.cpp

@@ -372,13 +372,12 @@ nsresult nsFileSpec::Execute(const nsString& args) const
 
 
 NS_IMETHODIMP
-nsLocalFile::MakeUnique(const char* suggestedName)
+nsLocalFile::CreateUnique(const char* suggestedName, PRUint32 type, PRUint32 attributes)
 {
-    PRBool exists;
-    nsresult rv = Exists(&exists);
+    nsresult rv = Create(type, attributes);
     
-    if (NS_FAILED(rv)) return rv;
-    if (!exists) return NS_OK;
+    if (NS_SUCCEEDED(rv)) return NS_OK;
+    if (rv != NS_ERROR_FILE_ALREADY_EXISTS) return rv;
 
     char* leafName; 
     rv = GetLeafName(&leafName);
@@ -386,10 +385,11 @@ nsLocalFile::MakeUnique(const char* suggestedName)
     if (NS_FAILED(rv)) return rv;
 
     char* lastDot = strrchr(leafName, '.');
-    char* suffix = "";
+    char suffix[kMaxFilenameLength + 1] = "";
     if (lastDot)
     {
-        suffix = nsCRT::strdup(lastDot); // include '.'
+        strncpy(suffix, lastDot, kMaxFilenameLength); // include '.'
+        suffix[kMaxFilenameLength] = 0; // make sure it's null terminated
         *lastDot = '\0'; // strip suffix and dot.
     }
 
@@ -399,17 +399,25 @@ nsLocalFile::MakeUnique(const char* suggestedName)
     if ((int)nsCRT::strlen(leafName) > (int)maxRootLength)
         leafName[maxRootLength] = '\0';
 
-    for (short indx = 1; indx < 10000 && exists; indx++)
+    for (short indx = 1; indx < 10000; indx++)
     {
         // start with "Picture-1.jpg" after "Picture.jpg" exists
         char newName[kMaxFilenameLength + 1];
         sprintf(newName, "%s-%d%s", leafName, indx, suffix);
         SetLeafName(newName);
 
-        rv = Exists(&exists);
-        if (NS_FAILED(rv)) return rv;
+        rv = Create(type, attributes);
+    
+        if (NS_SUCCEEDED(rv) || rv != NS_ERROR_FILE_ALREADY_EXISTS) 
+        {
+            nsMemory::Free(leafName);
+            return rv;
+        }
     }
-    return NS_OK;
+ 
+    nsMemory::Free(leafName);
+    // The disk is full, sort of
+    return NS_ERROR_FILE_TOO_BIG;
 }
 
 

xpcom/io/nsLocalFileWin.cpp

@@ -78,6 +78,11 @@ static nsresult ConvertWinError(DWORD winErr)
         case ERROR_HANDLE_DISK_FULL:
             rv = NS_ERROR_FILE_TOO_BIG;
             break;
+        case ERROR_FILE_EXISTS:
+        case ERROR_ALREADY_EXISTS:
+        case ERROR_CANNOT_MAKE:
+            rv = NS_ERROR_FILE_ALREADY_EXISTS;
+            break;
         case 0:
             rv = NS_OK;
         default:    
@@ -676,8 +681,10 @@ nsLocalFile::Create(PRUint32 type, PRUint32 attributes)
     {
         *slash = '\0';
 
-        CreateDirectoryA(mResolvedPath, NULL);// todo: pass back the result
-
+        if (!CreateDirectoryA(mResolvedPath, NULL)) {
+            rv = ConvertWinError(GetLastError());
+            if (rv != NS_ERROR_FILE_ALREADY_EXISTS) return rv;
+        }
         *slash = '\\';
         ++slash;
         slash = _mbschr(slash, '\\');
@@ -686,15 +693,19 @@ nsLocalFile::Create(PRUint32 type, PRUint32 attributes)
 
     if (type == NORMAL_FILE_TYPE)
     {
-        PRFileDesc* file = PR_Open(mResolvedPath, PR_RDONLY | PR_CREATE_FILE | PR_APPEND, attributes);
-        if (file) PR_Close(file);
+        PRFileDesc* file = PR_Open(mResolvedPath, PR_RDONLY | PR_CREATE_FILE | PR_APPEND | PR_EXCL, attributes);
+        if (!file) return NS_ERROR_FILE_ALREADY_EXISTS;
+
+        PR_Close(file);
         return NS_OK;
     }
 
     if (type == DIRECTORY_TYPE)
     {
-	    CreateDirectoryA(mResolvedPath, NULL); // todo: pass back the result
-        return NS_OK;
+        if (!CreateDirectoryA(mResolvedPath, NULL))
+            return ConvertWinError(GetLastError());
+        else 
+            return NS_OK;
     }
 
     return NS_ERROR_FILE_UNKNOWN_TYPE;