Restructure pam conversation function(Klockwork fix)
Altered return values form the function. With the earlier
implementation, the function returned PAM_AUTH_ERR on failure scenarios
which is incorrect. Replaced PAM_AUTH_ERR with PAM_CONV_ERR and
PAM_BUF_ERR at respetive places.
Added a check for number of messages received by the conversation
function capped at PAM_MAX_NUM_MSG.
Added a check for password size, which is capped at PAM_MAX_RESP_SIZE
as the bytes in the password greater than this limit would be discarded
by PAM.
Though pam_response structure and response, which are dynamically
allocated by the pam conversation function are the responsibility of the
caller to free them, with the current implemention, there is a possibility of
memory leak when numMsg would be zero or if PAM_PROMPT_ECHO_OFF
message never arrived.
This commit fixes the possible memory leak by allocating only on
receiving PAM_PROMPT_ECHO_OFF message.
Tested:
- ipmitool tool command passed with correct credentials:
- With Correct Creds: ipmitool -I lanplus -H x.x.x.x -C 17 -U root -P
0penBmc raw 0x00 0x01
Reply : 01 00 03 70
- With Incorrect Creds: ipmitool -I lanplus -H x.x.x.x -C 17 -U root
-P 0pen raw 0x00 0x01
Reply : Error: Unable to establish IPMI session
Signed-off-by: P Dheeraj Srujan Kumar <p.dheeraj.srujan.kumar@intel.com>
Change-Id: I670c3316eec01993a5cd0d79d1d6be248cf64328
diff --git a/user_channel/user_mgmt.cpp b/user_channel/user_mgmt.cpp
index 6b31eb9..182d17d 100644
--- a/user_channel/user_mgmt.cpp
+++ b/user_channel/user_mgmt.cpp
@@ -624,24 +624,56 @@
{
if (appdataPtr == nullptr)
{
- return PAM_AUTH_ERR;
+ return PAM_CONV_ERR;
}
- size_t passSize = std::strlen(reinterpret_cast<char*>(appdataPtr)) + 1;
- char* pass = reinterpret_cast<char*>(malloc(passSize));
- std::strncpy(pass, reinterpret_cast<char*>(appdataPtr), passSize);
- *resp = reinterpret_cast<pam_response*>(
- calloc(numMsg, sizeof(struct pam_response)));
+ if (numMsg <= 0 || numMsg >= PAM_MAX_NUM_MSG)
+ {
+ return PAM_CONV_ERR;
+ }
for (int i = 0; i < numMsg; ++i)
{
+ /* Ignore all PAM messages except prompting for hidden input */
if (msg[i]->msg_style != PAM_PROMPT_ECHO_OFF)
{
continue;
}
+
+ /* Assume PAM is only prompting for the password as hidden input */
+ /* Allocate memory only when PAM_PROMPT_ECHO_OFF is encounterred */
+
+ char* appPass = reinterpret_cast<char*>(appdataPtr);
+ size_t appPassSize = std::strlen(appPass);
+
+ if (appPassSize >= PAM_MAX_RESP_SIZE)
+ {
+ return PAM_CONV_ERR;
+ }
+
+ char* pass = reinterpret_cast<char*>(malloc(appPassSize + 1));
+ if (pass == nullptr)
+ {
+ return PAM_BUF_ERR;
+ }
+
+ void* ptr =
+ calloc(static_cast<size_t>(numMsg), sizeof(struct pam_response));
+ if (ptr == nullptr)
+ {
+ free(pass);
+ return PAM_BUF_ERR;
+ }
+
+ std::strncpy(pass, appPass, appPassSize + 1);
+
+ *resp = reinterpret_cast<pam_response*>(ptr);
resp[i]->resp = pass;
+
+ return PAM_SUCCESS;
}
- return PAM_SUCCESS;
+
+ return PAM_CONV_ERR;
}
/** @brief Updating the PAM password