rooty/yubico-pam
1
0
Fork 0
mirror of https://github.com/Yubico/yubico-pam.git synced 2024-12-12 03:08:50 +01:00
Code Issues Projects Releases Wiki Activity

Fix pam_get_data stack overwrite by saving a heap pointer instead

Browse Source 
The previous code was using a trick of saving the actual retval value
as the "pointer". The problem with that was when pam_get_data copied
it out it treated it as a void* which is 8 byte on 64 bit operating
system which meant it copied 8 byte to a 4 byte location and overwrote
the stack with 4 bytes.

The fix is using a heap pointer instead, influenced by the official
code in https://github.com/linux-pam/linux-pam/blob/master/modules/pam_unix/pam_unix_auth.c

With feedback from pedro martelletto, thanks.
This commit is contained in:
Gabriel Kihlman 2019-03-19 14:28:18 +01:00
parent eca00d0a58
commit 9531bc3c76
No known key found for this signature in database
GPG Key ID: C59A56181B3BF1F7
1 changed files with 26 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
pam_yubico.c
View File

@ -145,6 +145,13 @@ struct cfg
#endif #endif
#define DBG(x...) if (cfg->debug) { D(cfg->debug_file, x); } #define DBG(x...) if (cfg->debug) { D(cfg->debug_file, x); }
/* Helper to free memory used by pam_set_data */
static void
setcred_free (pam_handle_t *pamh, void *ptr, int err)
{
free (ptr);
}
/* /*
* Authorize authenticated OTP_ID for login as USERNAME using AUTHFILE. * Authorize authenticated OTP_ID for login as USERNAME using AUTHFILE.
* *
@ -1291,7 +1298,21 @@ done:
retval = PAM_SUCCESS; retval = PAM_SUCCESS;
} }
DBG ("done. [%s]", pam_strerror (pamh, retval)); DBG ("done. [%s]", pam_strerror (pamh, retval));
pam_set_data (pamh, "yubico_setcred_return", (void*)(intptr_t)retval, NULL);
int* pretval = malloc (sizeof(int));
if (pretval)
{
*pretval = retval;
if (pam_set_data (pamh, "yubico_setcred_return", (void*)pretval,
setcred_free) != PAM_SUCCESS)
{
DBG ("pam_set_data failed setting setcred_return: %d", retval);
}
}
else
{
DBG ("Failed allocating memory for setcred_return status");
}
if (resp) if (resp)
{ {
@ -1326,11 +1347,12 @@ pam_sm_acct_mgmt(pam_handle_t *pamh, int flags, int argc, const char **argv)
{ {
struct cfg cfg_st; struct cfg cfg_st;
struct cfg *cfg = &cfg_st; /* for DBG macro */ struct cfg *cfg = &cfg_st; /* for DBG macro */
int retval; int retval = PAM_AUTH_ERR;
int rc = pam_get_data(pamh, "yubico_setcred_return", (const void**)&retval); const void *pretval = NULL;
int rc = pam_get_data (pamh, "yubico_setcred_return", &pretval);
parse_cfg (flags, argc, argv, cfg); parse_cfg (flags, argc, argv, cfg);
if (rc == PAM_SUCCESS && retval == PAM_SUCCESS) { if (rc == PAM_SUCCESS && pretval && *(const int *)pretval == PAM_SUCCESS) {
DBG ("pam_sm_acct_mgmt returning PAM_SUCCESS"); DBG ("pam_sm_acct_mgmt returning PAM_SUCCESS");
retval = PAM_SUCCESS; retval = PAM_SUCCESS;
} else { } else {