[patch] slightly more careful patch

pypam-0.5.0-cve-2012-1502.patch (text/plain), 1.50 KB, created by Marien Zwart (RETIRED) on 2012-03-10 01:24:41 UTC

Only touch the "resp" pam_conv argument when that function succeeds.
Leaving it a dangling pointer can be harmful as some implementations
will try to free it for us.

Patch by [e-mail].

--- PAMmodule.c
+++ PAMmodule.c
@@ -82,17 +82,23 @@
         Py_DECREF(respList);
         return PAM_CONV_ERR;
     }
-    
-    *resp = (struct pam_response *) malloc(
+
+    /* pam_conv(3) says we "should not set *resp" on PAM_CONV_ERR.
+     * Some pam implementations will free it if we set it, others will not.
+     * So at the very least we need to not leave it dangling if we error out,
+     * but for robustness just only set it if we will exit successfully.
+     */
+
+    struct pam_response* myresp = (struct pam_response *) malloc(
         PyList_Size(respList) * sizeof(struct pam_response));
 
-    struct pam_response* spr = *resp;
+    struct pam_response* spr = myresp;
     for (int i = 0; i < PyList_Size(respList); i++, spr++) {
         PyObject* respTuple = PyList_GetItem(respList, i);
         char* resp_text;
         int resp_retcode = 0;
         if (!PyArg_ParseTuple(respTuple, "si", &resp_text, &resp_retcode)) {
-            free(*resp);
+            free(myresp);
             Py_DECREF(respList);
             return PAM_CONV_ERR;
         }
@@ -100,7 +106,8 @@
         spr->resp_retcode = resp_retcode;
         Py_DECREF(respTuple);
     }
-    
+
+    *resp = myresp;
     // Save this so we can free it later.
     self->response_data = *resp;
     self->response_len = PyList_Size(respList);