From ec8076dc1abe3adf1738f561b97f727093154cc9 Mon Sep 17 00:00:00 2001 From: Marcin Lis Date: Thu, 29 Aug 2013 12:10:17 +0200 Subject: [PATCH] Fix security server API argument validation [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 | 22 +++++++++++++++++++++- src/server2/client/client-privilege-by-pid.cpp | 17 +++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/server/security-server-password.c b/src/server/security-server-password.c index a8fa836..e20233f 100644 --- a/src/server/security-server-password.c +++ b/src/server/security-server-password.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -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); diff --git a/src/server2/client/client-privilege-by-pid.cpp b/src/server2/client/client-privilege-by-pid.cpp index ff6ddd9..cc35122 100644 --- a/src/server2/client/client-privilege-by-pid.cpp +++ b/src/server2/client/client-privilege-by-pid.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include @@ -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)); -- 2.7.4