fix(vfs): Fix TOCTOU vulnerability

Author: lucasssvaz

libraries/FS/src/vfs_api.cpp

@@ -13,6 +13,9 @@
 // limitations under the License.
 
 #include "vfs_api.h"
+#include <errno.h>
+#include <stdlib.h>
+#include <string.h>
 
 using namespace fs;
 
@@ -38,8 +41,47 @@ FileImplPtr VFSImpl::open(const char *fpath, const char *mode, const bool create
   strcpy(temp, _mountpoint);
   strcat(temp, fpath);
 
+  // Try to open as file first - let the file operation handle errors
+  if (mode && mode[0] != 'r') {
+    // For write modes, attempt to create directories if needed
+    if (create) {
+      char *token;
+      char *folder = (char *)malloc(strlen(fpath) + 1);
+
+      int start_index = 0;
+      int end_index = 0;
+
+      token = strchr(fpath + 1, '/');
+      end_index = (token - fpath);
+
+      while (token != NULL) {
+        memcpy(folder, fpath + start_index, end_index - start_index);
+        folder[end_index - start_index] = '\0';
+
+        if (!VFSImpl::mkdir(folder)) {
+          log_e("Creating folder: %s failed!", folder);
+          free(folder);
+          free(temp);
+          return FileImplPtr();
+        }
+
+        token = strchr(token + 1, '/');
+        if (token != NULL) {
+          end_index = (token - fpath);
+          memset(folder, 0, strlen(folder));
+        }
+      }
+
+      free(folder);
+    }
+
+    // Try to open the file directly - let fopen handle errors
+    free(temp);
+    return std::make_shared<VFSFileImpl>(this, fpath, mode);
+  }
+
+  // For read mode, try to open as file first
   struct stat st;
-  //file found
   if (!stat(temp, &st)) {
     free(temp);
     if (S_ISREG(st.st_mode) || S_ISDIR(st.st_mode)) {
@@ -49,54 +91,15 @@ FileImplPtr VFSImpl::open(const char *fpath, const char *mode, const bool create
     return FileImplPtr();
   }
 
-  //try to open this as directory (might be mount point)
+  // Try to open as directory (might be mount point)
   DIR *d = opendir(temp);
   if (d) {
     closedir(d);
     free(temp);
     return std::make_shared<VFSFileImpl>(this, fpath, mode);
   }
 
-  //file not found but mode permits file creation without folder creation
-  if ((mode && mode[0] != 'r') && (!create)) {
-    free(temp);
-    return std::make_shared<VFSFileImpl>(this, fpath, mode);
-  }
-
-  ////file not found but mode permits file creation and folder creation
-  if ((mode && mode[0] != 'r') && create) {
-
-    char *token;
-    char *folder = (char *)malloc(strlen(fpath) + 1);
-
-    int start_index = 0;
-    int end_index = 0;
-
-    token = strchr(fpath + 1, '/');
-    end_index = (token - fpath);
-
-    while (token != NULL) {
-      memcpy(folder, fpath + start_index, end_index - start_index);
-      folder[end_index - start_index] = '\0';
-
-      if (!VFSImpl::mkdir(folder)) {
-        log_e("Creating folder: %s failed!", folder);
-        return FileImplPtr();
-      }
-
-      token = strchr(token + 1, '/');
-      if (token != NULL) {
-        end_index = (token - fpath);
-        memset(folder, 0, strlen(folder));
-      }
-    }
-
-    free(folder);
-    free(temp);
-    return std::make_shared<VFSFileImpl>(this, fpath, mode);
-  }
-
-  log_e("%s does not exist, no permits for creation", temp);
+  // File not found and read mode - return empty
   free(temp);
   return FileImplPtr();
 }
@@ -125,10 +128,7 @@ bool VFSImpl::rename(const char *pathFrom, const char *pathTo) {
     log_e("bad arguments");
     return false;
   }
-  if (!exists(pathFrom)) {
-    log_e("%s does not exists", pathFrom);
-    return false;
-  }
+
   size_t mountpointLen = strlen(_mountpoint);
   char *temp1 = (char *)malloc(strlen(pathFrom) + mountpointLen + 1);
   if (!temp1) {
@@ -148,6 +148,7 @@ bool VFSImpl::rename(const char *pathFrom, const char *pathTo) {
   strcpy(temp2, _mountpoint);
   strcat(temp2, pathTo);
 
+  // Let rename() handle the error if source doesn't exist
   auto rc = ::rename(temp1, temp2);
   free(temp1);
   free(temp2);
@@ -165,16 +166,6 @@ bool VFSImpl::remove(const char *fpath) {
     return false;
   }
 
-  VFSFileImpl f(this, fpath, "r");
-  if (!f || f.isDirectory()) {
-    if (f) {
-      f.close();
-    }
-    log_e("%s does not exists or is directory", fpath);
-    return false;
-  }
-  f.close();
-
   char *temp = (char *)malloc(strlen(fpath) + strlen(_mountpoint) + 1);
   if (!temp) {
     log_e("malloc failed");
@@ -184,6 +175,7 @@ bool VFSImpl::remove(const char *fpath) {
   strcpy(temp, _mountpoint);
   strcat(temp, fpath);
 
+  // Let unlink() handle the error if file doesn't exist
   auto rc = unlink(temp);
   free(temp);
   return rc == 0;
@@ -195,17 +187,6 @@ bool VFSImpl::mkdir(const char *fpath) {
     return false;
   }
 
-  VFSFileImpl f(this, fpath, "r");
-  if (f && f.isDirectory()) {
-    f.close();
-    //log_w("%s already exists", fpath);
-    return true;
-  } else if (f) {
-    f.close();
-    log_e("%s is a file", fpath);
-    return false;
-  }
-
   char *temp = (char *)malloc(strlen(fpath) + strlen(_mountpoint) + 1);
   if (!temp) {
     log_e("malloc failed");
@@ -215,9 +196,15 @@ bool VFSImpl::mkdir(const char *fpath) {
   strcpy(temp, _mountpoint);
   strcat(temp, fpath);
 
+  // Let mkdir() handle the error if directory already exists
   auto rc = ::mkdir(temp, ACCESSPERMS);
   free(temp);
-  return rc == 0;
+
+  // EEXIST means directory already exists, which is fine
+  if (rc == 0 || errno == EEXIST) {
+    return true;
+  }
+  return false;
 }
 
 bool VFSImpl::rmdir(const char *fpath) {
@@ -231,16 +218,6 @@ bool VFSImpl::rmdir(const char *fpath) {
     return false;
   }
 
-  VFSFileImpl f(this, fpath, "r");
-  if (!f || !f.isDirectory()) {
-    if (f) {
-      f.close();
-    }
-    log_e("%s does not exists or is a file", fpath);
-    return false;
-  }
-  f.close();
-
   char *temp = (char *)malloc(strlen(fpath) + strlen(_mountpoint) + 1);
   if (!temp) {
     log_e("malloc failed");
@@ -250,6 +227,7 @@ bool VFSImpl::rmdir(const char *fpath) {
   strcpy(temp, _mountpoint);
   strcat(temp, fpath);
 
+  // Let rmdir() handle the error if directory doesn't exist
   auto rc = ::rmdir(temp);
   free(temp);
   return rc == 0;
@@ -271,8 +249,27 @@ VFSFileImpl::VFSFileImpl(VFSImpl *fs, const char *fpath, const char *mode) : _fs
     return;
   }
 
+  // Try to open as file first
+  _f = fopen(temp, mode);
+  if (_f) {
+    _isDirectory = false;
+    if (_stat.st_blksize == 0) {
+      setvbuf(_f, NULL, _IOFBF, DEFAULT_FILE_BUFFER_SIZE);
+    }
+    free(temp);
+    return;
+  }
+
+  // Try to open as directory
+  _d = opendir(temp);
+  if (_d) {
+    _isDirectory = true;
+    free(temp);
+    return;
+  }
+
+  // If neither worked, try to stat to determine type
   if (!stat(temp, &_stat)) {
-    //file found
     if (S_ISREG(_stat.st_mode)) {
       _isDirectory = false;
       _f = fopen(temp, mode);
@@ -292,18 +289,8 @@ VFSFileImpl::VFSFileImpl(VFSImpl *fs, const char *fpath, const char *mode) : _fs
       log_e("Unknown type 0x%08X for file %s", ((_stat.st_mode) & _IFMT), temp);
     }
   } else {
-    //file not found
-    if (!mode || mode[0] == 'r') {
-      //try to open as directory
-      _d = opendir(temp);
-      if (_d) {
-        _isDirectory = true;
-      } else {
-        _isDirectory = false;
-        //log_w("stat(%s) failed", temp);
-      }
-    } else {
-      //lets create this new file
+    // File not found - this is normal for write modes
+    if (mode && mode[0] != 'r') {
       _isDirectory = false;
       _f = fopen(temp, mode);
       if (!_f) {