Avoid to use stat() function

Doing something after checking of the state of a file has a potential
race condition bug. It is more effiecient that just using execption
handling instead of checking to prevent TOCTOU.

Change-Id: I2a2560d9d49ed9d52008557f0d2f89fa04bb130d

diff --git a/packaging/download-provider.spec b/packaging/download-provider.spec
index 95639d0..e7f60aa 100755 (executable)
--- a/packaging/download-provider.spec
+++ b/packaging/download-provider.spec
@@ -1,6 +1,6 @@
 Name:       download-provider
 Summary:    Download the contents in background
-Version:    2.2.4
+Version:    2.2.5
 Release:    0
 Group:      Development/Libraries
 License:    Apache-2.0

diff --git a/provider/download-provider-notify.c b/provider/download-provider-notify.c
index e2d541f..62d2df7 100644 (file)
--- a/provider/download-provider-notify.c
+++ b/provider/download-provider-notify.c
@@ -19,6 +19,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
+#include <errno.h>
 
 #include <download-provider-log.h>
 #include <download-provider-client.h>
@@ -47,9 +48,10 @@ int dp_notify_init(pid_t pid)
        if (notify_fifo == NULL)
                return -1;
        int notify_fd = -1;
-       struct stat fifo_state;
-       if (stat(notify_fifo, &fifo_state) == 0) // found
-               unlink(notify_fifo);
+
+       if (unlink(notify_fifo) < 0)
+               TRACE_ERROR("%s might not be exist. Ignored. errno(%d)", notify_fifo, errno);
+
        if (mkfifo(notify_fifo, 0644/*-rwrr*/) < 0)
                TRACE_ERROR("failed to make fifo %s", notify_fifo);
        else
@@ -64,9 +66,10 @@ void dp_notify_deinit(pid_t pid)
        char *notify_fifo = __dp_notify_get_path(pid);
        if (notify_fifo == NULL)
                return ;
-       struct stat fifo_state;
-       if (stat(notify_fifo, &fifo_state) == 0) // found
-               unlink(notify_fifo);
+
+       if (unlink(notify_fifo) < 0)
+               TRACE_ERROR("%s might not be exist. Ignored. errno(%d)", notify_fifo, errno);
+
        free(notify_fifo);
 }
 

diff --git a/provider/download-provider-plugin-download-agent.c b/provider/download-provider-plugin-download-agent.c
index 961baa5..76796f3 100755 (executable)
--- a/provider/download-provider-plugin-download-agent.c
+++ b/provider/download-provider-plugin-download-agent.c
@@ -202,52 +202,48 @@ static int __precheck_request(dp_client_slots_fmt* slot, dp_request_fmt *request
 
 static int __set_file_permission_to_client(dp_client_slots_fmt *slot, dp_request_fmt *request, char *saved_path)
 {
-       if (slot == NULL || request == NULL) {
-               TRACE_ERROR("check address slot:%p request:%p id:%d agentid:%d", slot, request, (request == NULL ? 0 : request->id), (request == NULL ? 0 : request->agent_id));
-               return DP_ERROR_INVALID_PARAMETER;
-       } else if (saved_path == NULL) {
-               TRACE_ERROR("check null saved path");
+       if (slot == NULL || request == NULL || saved_path == NULL) {
+               TRACE_ERROR("slot:%p request:%p id:%d agentid:%d saved_path:%s",
+                               slot, request, (request == NULL ? 0 : request->id),
+                               (request == NULL ? 0 : request->agent_id), saved_path ? saved_path : "");
                return DP_ERROR_INVALID_PARAMETER;
        }
 
        struct stat lstat_info;
-       struct stat fstat_info;
        int fd;
-       int errorcode = DP_ERROR_NONE;
        dp_credential cred = slot->credential;
-       if (lstat(saved_path, &lstat_info) != -1) {
-               fd = open(saved_path, O_RDONLY);
-               if (fd != -1) {
-                       if (fstat(fd, &fstat_info) != -1) {
-                               if (lstat_info.st_mode == fstat_info.st_mode &&
-                                               lstat_info.st_ino == fstat_info.st_ino &&
-                                               lstat_info.st_dev == fstat_info.st_dev) {
-
-                                       if (strncmp(saved_path, "/opt/media/", 11) != 0) {
-                                               if (fchown(fd, cred.uid, cred.gid) != 0) {
-                                                       TRACE_ERROR("[ERROR][%d] permission user:%d group:%d",
-                                                                       request->id, cred.uid, cred.gid);
-                                                       errorcode = DP_ERROR_PERMISSION_DENIED;
-                                               }
-                                       }
-                               } else {
-                                       TRACE_ERROR("fstat & lstat info have not matched");
-                                       errorcode = DP_ERROR_PERMISSION_DENIED;
-                               }
-                       } else {
-                               TRACE_ERROR("fstat call failed");
-                               errorcode = DP_ERROR_PERMISSION_DENIED;
-                       }
-                       close(fd);
-               } else {
-                       TRACE_SECURE_ERROR("open failed for file : %s", saved_path);
-                       errorcode = DP_ERROR_IO_ERROR;
-               }
-       } else {
-               TRACE_ERROR("lstat call failed");
-               errorcode = DP_ERROR_PERMISSION_DENIED;
+
+       if (lstat(saved_path, &lstat_info) == -1) {
+               TRACE_ERROR("Cannot access to %s", saved_path);
+               return DP_ERROR_PERMISSION_DENIED;
        }
-       return errorcode;
+
+       if ((lstat_info.st_mode & S_IFMT) == S_IFLNK) {
+               TRACE_ERROR("%s is a symbolic link.", saved_path);
+               return DP_ERROR_PERMISSION_DENIED;
+       }
+
+       fd = open(saved_path, O_RDONLY);
+       if (fd == -1) {
+               TRACE_SECURE_ERROR("open failed for file : %s", saved_path);
+               return DP_ERROR_IO_ERROR;
+       }
+
+       // Don't need to change the owner and group of a file.
+       if (strncmp(saved_path, "/opt/media/", 11) != 0) {
+               close(fd);
+               return DP_ERROR_NONE;
+       }
+
+       if (fchown(fd, cred.uid, cred.gid) != 0) {
+               TRACE_ERROR("[ERROR][%d] permission user:%d group:%d",
+                               request->id, cred.uid, cred.gid);
+               close(fd);
+               return DP_ERROR_PERMISSION_DENIED;
+       }
+
+       close(fd);
+       return DP_ERROR_NONE;
 }
 
 static void __finished_cb(finished_info_t *info, void *user_req_data,