Fix static analysis issue

Fix TOCTOU problem.

Change-Id: I0c0591a86a7a86a9df6ba34bc629ca0010cf192f
Signed-off-by: Sangyoon Jang <jeremy.jang@samsung.com>

diff --git a/src/common/utils/file_logbackend.cc b/src/common/utils/file_logbackend.cc
index 0676f88..279e7f5 100644 (file)
--- a/src/common/utils/file_logbackend.cc
+++ b/src/common/utils/file_logbackend.cc
@@ -5,6 +5,7 @@
 #include "common/utils/file_logbackend.h"
 
 #include <manifest_parser/utils/logging.h>
+#include <fcntl.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <unistd.h>
@@ -23,9 +24,9 @@ constexpr mode_t kDefaultMode640 = S_IRUSR | S_IWUSR | S_IRGRP;
 
 namespace utils {
 
-FileLogBackend::FileLogBackend(std::string file_name, int rotation_size,
+FileLogBackend::FileLogBackend(std::string file_path, int rotation_size,
     int max_rotation)
-    : file_name_(std::move(file_name)), rotation_size_(rotation_size),
+    : file_path_(std::move(file_path)), rotation_size_(rotation_size),
       max_rotation_(max_rotation), log_stream_(
           std::unique_ptr<std::ostringstream>(new std::ostringstream())) {
 }
@@ -38,15 +39,15 @@ void FileLogBackend::WriteLog(LogLevel level, const std::string& /* tag */,
 }
 
 void FileLogBackend::WriteLogToFile() {
-  if (file_name_.empty())
+  if (file_path_.empty())
     return;
 
-  int size = GetFileSize(file_name_);
+  int size = GetFileSize(file_path_);
   if (size > rotation_size_)
     if (!Rotate())
       return;
 
-  std::ofstream ofs(file_name_.c_str(), std::ios::app);
+  std::ofstream ofs(file_path_.c_str(), std::ios::app);
   ofs << log_stream_->str();
   ofs.close();
 
@@ -54,31 +55,44 @@ void FileLogBackend::WriteLogToFile() {
   log_stream_->str("");
   log_stream_->clear();
 
-  if (chmod(file_name_.c_str(), kDefaultMode640) != 0)
+  if (chmod(file_path_.c_str(), kDefaultMode640) != 0)
     LOG(ERROR) << "Failed to set permission on log, errno : " << errno;
 }
 
 bool FileLogBackend::Rotate() {
+  std::string logdir = GetLogDir();
+  int dirfd = open(logdir.c_str(), O_DIRECTORY);
   for (int i = max_rotation_; i > 0; i--) {
-    std::string old_log = file_name_ + "." + std::to_string(i);
+    std::string old_log = GetFileName() + "." + std::to_string(i);
 
+    if (dirfd < 0) {
+      LOG(ERROR) << "Failed to open dir(" << logdir << "), errno : " << errno;
+      return false;
+    }
     struct stat tmp_buffer;
-    if (stat(old_log.c_str(), &tmp_buffer) != 0)
+    if (fstatat(dirfd, old_log.c_str(), &tmp_buffer, 0) != 0)
       continue;
 
     // the oldest log will be removed
     if (i == max_rotation_) {
-      if (std::remove(old_log.c_str()) != 0)
+      if (unlinkat(dirfd, old_log.c_str(), 0) != 0) {
+        close(dirfd);
         return false;
+      }
     } else {
-      std::string new_log = file_name_ + "." + std::to_string(i + 1);
-      if (std::rename(old_log.c_str(), new_log.c_str()) != 0)
+      std::string new_log = GetFileName() + "." + std::to_string(i + 1);
+      if (renameat(dirfd, old_log.c_str(), dirfd, new_log.c_str()) != 0) {
+        close(dirfd);
         return false;
+      }
     }
   }
-  std::string new_log = file_name_ + ".1";
-  if (std::rename(file_name_.c_str(), new_log.c_str()) != 0)
+  std::string new_log = GetFileName() + ".1";
+  if (renameat(dirfd, file_path_.c_str(), dirfd, new_log.c_str()) != 0) {
+    close(dirfd);
     return false;
+  }
+  close(dirfd);
 
   return true;
 }
@@ -111,4 +125,12 @@ std::string FileLogBackend::GetPid() {
   return std::to_string(getpid()) + "|";
 }
 
+std::string FileLogBackend::GetLogDir() {
+  return file_path_.substr(0, file_path_.find_last_of("\\/") + 1);
+}
+
+std::string FileLogBackend::GetFileName() {
+  return file_path_.substr(file_path_.find_last_of("\\/") + 1);
+}
+
 }  // namespace utils

diff --git a/src/common/utils/file_logbackend.h b/src/common/utils/file_logbackend.h
index 1bfb6ce..fd5f099 100644 (file)
--- a/src/common/utils/file_logbackend.h
+++ b/src/common/utils/file_logbackend.h
@@ -15,18 +15,20 @@ namespace utils {
 
 class FileLogBackend : public ILogBackend {
  public:
-  FileLogBackend(std::string file_name, int rotation_size, int max_rotation);
+  FileLogBackend(std::string file_path, int rotation_size, int max_rotation);
   void WriteLog(LogLevel level, const std::string& tag,
       const std::string& logstr) override;
   void WriteLogToFile();
 
  private:
   bool Rotate();
-  int GetFileSize(const std::string& file_name);
+  int GetFileSize(const std::string& file_path);
   std::string GetTimeStamp();
   std::string GetPid();
+  std::string GetLogDir();
+  std::string GetFileName();
 
-  std::string file_name_;
+  std::string file_path_;
   int rotation_size_;
   int max_rotation_;
   std::unique_ptr<std::ostringstream> log_stream_;