Fix security server API argument validation
authorMarcin Lis <m.lis@samsung.com>
Thu, 29 Aug 2013 10:10:17 +0000 (12:10 +0200)
committerBartlomiej Grzelewski <b.grzelewski@samsung.com>
Thu, 6 Feb 2014 16:13:22 +0000 (17:13 +0100)
[Issue#]       SSDWSSP-332
[Bug]          Potential bug, bad parameters (f.e. pid) may be used in two
               functions.
[Cause]        Two API functions does not validate all given arguments.
               Also there is a risk of unsigned int overflow when calculating
               password validity (when changing from days to seconds).
[Solution]     Checking existence of a process with given pid added.
               Unsigned int overflow protection is added.
               Also argument validation added to API function.
[Verification] Build, install and run tests.

Change-Id: Ia8e9528462e31220faf88c72c111b7f0efc03681

src/server/security-server-password.c
src/server2/client/client-privilege-by-pid.cpp

index a8fa836..e20233f 100644 (file)
@@ -24,6 +24,7 @@
 #include <string.h>
 #include <dirent.h>
 #include <errno.h>
+#include <limits.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <time.h>
@@ -948,6 +949,7 @@ int process_set_pwd_request(int sockfd)
         expire_time = 0;
     else
     {
+        /* Check if value converted to seconds will not exceed the range of unsigned int */
         time_t t = time(NULL );
         unsigned int valid_days_max = (UINT_MAX - t) / 86400;
         if (valid_days > valid_days_max)
@@ -1118,7 +1120,25 @@ int process_reset_pwd_request(int sockfd)
     if (valid_days == 0)
         expire_time = 0;
     else
-        expire_time = time(NULL) + (valid_days * 86400);
+    {
+        /* Check if value converted to seconds will not exceed the range of unsigned int */
+        time_t t = time(NULL );
+        unsigned int valid_days_max = (UINT_MAX - t) / 86400;
+        if (valid_days > valid_days_max)
+        {
+            SECURE_SLOGE("%s",
+                    "Server: Max password validity exceeded (%d>%d)", valid_days, valid_days_max);
+            retval = send_generic_response(sockfd,
+                    SECURITY_SERVER_MSG_TYPE_SET_PWD_RESPONSE,
+                    SECURITY_SERVER_RETURN_CODE_BAD_REQUEST);
+            if (retval != SECURITY_SERVER_SUCCESS)
+            {
+                SEC_SVR_ERR("Server ERROR: Cannot send generic response: %d", retval);
+            }
+            goto error;
+        }
+        expire_time = t + (valid_days * 86400);
+    }
 
     /* Hash requested password */
     SHA256_Init(&context);
index ff6ddd9..cc35122 100644 (file)
@@ -32,6 +32,7 @@
 #include <client-common.h>
 #include <protocols.h>
 #include <smack-check.h>
+#include <signal.h>
 
 #include <security-server.h>
 #include <security-server-common.h>
@@ -46,6 +47,22 @@ int security_server_check_privilege_by_pid(
         if (1 != smack_check())
             return SECURITY_SERVER_API_SUCCESS;
 
+        // Checking whether a process with pid exists
+        if ((pid < 0) || ((kill(pid, 0) == -1) && (errno == ESRCH))) {
+            LogDebug("pid is invalid, process: " << pid << " does not exist");
+            return SECURITY_SERVER_API_ERROR_INPUT_PARAM;
+        }
+
+        if (NULL == object || 0 == strlen(object)) {
+            LogDebug("object param is NULL or empty");
+            return SECURITY_SERVER_API_ERROR_INPUT_PARAM;
+        }
+
+        if (NULL == access_rights || 0 == strlen(access_rights)) {
+            LogDebug("access_right param is NULL or empty");
+            return SECURITY_SERVER_API_ERROR_INPUT_PARAM;
+        }
+
         SocketBuffer send, recv;
         Serialization::Serialize(send, pid);
         Serialization::Serialize(send, std::string(object));