From 9531bc3c76f1e985ed8604c03591536dac39028a Mon Sep 17 00:00:00 2001
From: Gabriel Kihlman <g.kihlman@yubico.com>
Date: Tue, 19 Mar 2019 14:28:18 +0100
Subject: [PATCH] Fix pam_get_data stack overwrite by saving a heap pointer
 instead

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.
---
 pam_yubico.c | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/pam_yubico.c b/pam_yubico.c
index 30ee6ec..f8ba801 100644
--- a/pam_yubico.c
+++ b/pam_yubico.c
@@ -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.
  *
@@ -1291,7 +1298,21 @@ done:
       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)
     {
@@ -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 {