Avoid double free of ctx->princ in a failure case

When re-retrieving the authenticated principal from the current cache,
ensure the stored principal in the authentication context is always
either valid or NULL.  Otherwise, a failure of krb5_cc_get_principal
could result in a double free.  Thanks to Michael Muehle for the
report.

Fixes #20

diff --git a/NEWS b/NEWS
index 4e1a0ad96419652001863d2cdd0d82ce4e8ef16a..c395031b291445ee51d18154578a3a06b597c4a1 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,13 @@
                       User-Visible pam-krb5 Changes
 
+pam-krb5 4.10 (unreleased)
+
+    When re-retrieving the authenticated principal from the current cache,
+    ensure the stored principal in the authentication context is always
+    either valid or NULL.  Otherwise, a failure of krb5_cc_get_principal
+    could result in a double free.  Thanks to Michael Muehle for the
+    report.
+
 pam-krb5 4.9 (2020-03-30)
 
     SECURITY: All previous versions of this module could overflow the

diff --git a/module/account.c b/module/account.c
index 211975a43ac5e882b33c6758d3aec3af4774f434..c270c9b974310c9a94dd0a3d777007e1fb7e6eb1 100644 (file)
--- a/module/account.c
+++ b/module/account.c
@@ -5,7 +5,7 @@
  * user's authorization against .k5login (or whatever equivalent we've been
  * configured for).
  *
- * Copyright 2005-2009, 2014, 2020 Russ Allbery <eagle@eyrie.org>
+ * Copyright 2005-2009, 2014, 2020-2021 Russ Allbery <eagle@eyrie.org>
  * Copyright 2011
  *     The Board of Trustees of the Leland Stanford Junior University
  * Copyright 2005 Andres Salomon <dilinger@debian.org>
@@ -78,8 +78,10 @@ pamk5_account(struct pam_args *args)
      */
     if (ctx->cache != NULL) {
         putil_debug(args, "retrieving principal from cache");
-        if (ctx->princ != NULL)
+        if (ctx->princ != NULL) {
             krb5_free_principal(ctx->context, ctx->princ);
+            ctx->princ = NULL;
+        }
         retval = krb5_cc_get_principal(ctx->context, ctx->cache, &ctx->princ);
         if (retval != 0) {
             putil_err_krb5(args, retval, "cannot get principal from cache");