Skip to content

Commit

Permalink
Fix pam_get_data stack overwrite by saving a heap pointer instead
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Gabriel Kihlman committed Mar 19, 2019
1 parent eca00d0 commit 9531bc3
Showing 1 changed file with 26 additions and 4 deletions.
30 changes: 26 additions & 4 deletions pam_yubico.c
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,13 @@ struct cfg
#endif
#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.
*
Expand Down Expand Up @@ -1291,7 +1298,21 @@ pam_sm_authenticate (pam_handle_t * pamh,
retval = PAM_SUCCESS;
}
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)
{
Expand Down Expand Up @@ -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 = &cfg_st; /* for DBG macro */
int retval;
int rc = pam_get_data(pamh, "yubico_setcred_return", (const void**)&retval);
int retval = PAM_AUTH_ERR;
const void *pretval = NULL;
int rc = pam_get_data (pamh, "yubico_setcred_return", &pretval);

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");
retval = PAM_SUCCESS;
} else {
Expand Down

0 comments on commit 9531bc3

Please sign in to comment.