Fix princflags memory management

Fix some out of memory error cases (found by Coverity) that could
cause multiple frees or freeing of invalid pointers.  In
krb5_flagnum_to_string(), don't assume that asprintf() stores a null
pointer on failure (it does in BSD but not in glibc).  In
krb5_flags_to_strings(), free the correct pointer in the cleanup loop
in on error.

ticket: 8215

diff --git a/src/lib/kadm5/str_conv.c b/src/lib/kadm5/str_conv.c
index fd9a1d178afbae98ca4b7794389d635216e0915c..0441a17682aff6f55c4a24b40ffe6688e30c4b0e 100644 (file)
--- a/src/lib/kadm5/str_conv.c
+++ b/src/lib/kadm5/str_conv.c
@@ -196,7 +196,8 @@ krb5_flagspec_to_mask(const char *spec, krb5_flags *toset, krb5_flags *toclear)
 }
 
 /*
- * Copy the flag name of flagnum to outstr.
+ * Copy the flag name of flagnum to outstr.  On error, outstr points to a null
+ * pointer.
  */
 krb5_error_code
 krb5_flagnum_to_string(int flagnum, char **outstr)
@@ -204,14 +205,15 @@ krb5_flagnum_to_string(int flagnum, char **outstr)
     const char *s = NULL;
 
     *outstr = NULL;
-    if ((unsigned int)flagnum < NOUTFLAGS) {
+    if ((unsigned int)flagnum < NOUTFLAGS)
         s = outflags[flagnum];
-    }
-    if (s == NULL)
+    if (s == NULL) {
         /* Assume that krb5_flags are 32 bits long. */
-        asprintf(outstr, "0x%08lx", 1UL<<flagnum);
-    else
+        if (asprintf(outstr, "0x%08lx", 1UL << flagnum) == -1)
+            *outstr = NULL;
+    } else {
         *outstr = strdup(s);
+    }
     if (*outstr == NULL)
         return ENOMEM;
     return 0;
@@ -242,15 +244,15 @@ krb5_flags_to_strings(krb5_int32 flags, char ***outarray)
         }
         a = a_new;
         retval = krb5_flagnum_to_string(i, &a[amax++]);
+        a[amax] = NULL;
         if (retval)
             goto cleanup;
-        a[amax] = NULL;
     }
     *outarray = a;
     return 0;
 cleanup:
     for (ap = a; ap != NULL && *ap != NULL; ap++) {
-        free(ap);
+        free(*ap);
     }
     free(a);
     return retval;

diff --git a/src/tests/t_princflags.py b/src/tests/t_princflags.py
index 03817bf093d41be9a1c91a9a268dca3bac7c7e3f..6378ef94fce2d52adf8e182ddb2271e8f32e5a81 100755 (executable)
--- a/src/tests/t_princflags.py
+++ b/src/tests/t_princflags.py
@@ -103,6 +103,18 @@ def one_aclcheck(ftuple, doset):
             fail('Failed to keep flag ' + outname + ' clear')
 
 
+# Set all flags simultaneously, even the ones that aren't defined yet.
+def lamptest():
+    pat = re.compile('^Attributes: ' +
+                     ' '.join(flags2namelist(0xffffffff)) +
+                     '$', re.MULTILINE)
+    realm.run([kadminl, 'ank', '-pw', 'password', '+0xffffffff', 'test'])
+    out = realm.run([kadminl, 'getprinc', 'test'])
+    if not pat.search(out):
+        fail('Failed to simultaenously set all flags')
+    realm.run([kadminl, 'delprinc', 'test'])
+
+
 for ftuple in kadmin_ftuples:
     one_kadmin_flag(ftuple)
 
@@ -122,5 +134,6 @@ for ftuple in strconv_ftuples:
     one_aclcheck(ftuple, True)
     one_aclcheck(ftuple, False)
 
+lamptest()
 
 success('KDB principal flags')