pts: add some sanity checks in ptuser.c
[openafs.git] / src / ptserver / ptuser.c
index 4296149..7df9291 100644 (file)
@@ -332,22 +332,40 @@ pr_End(void)
     return code;
 }
 
-
+/*
+ * Make sure that arg is a proper C string that fits in a prname.
+ * If strnlen(arg, PR_MAXNAMELEN) == PR_MAXNAMELEN, then arg either
+ * doesn't have a terminating NUL or is too long, and we can't tell
+ * which one in the current API.  This code has always assumed that
+ * the names presented to it are valid C strings, but for robustness
+ * we can't depend on the server side guaranteeing that.  Unfortunately,
+ * the wire protocol uses a vector[PR_MAXNAMELEN] of char, so XDR will
+ * not automatically fix up strings generated by the server.
+ *
+ * The inequality is just belt-and-suspenders and should be impossible.
+ */
+static_inline int check_length(prname arg)
+{
+    if (strnlen(arg, PR_MAXNAMELEN) >= PR_MAXNAMELEN)
+       return PRNAMETOOLONG;
+    return 0;
+}
 
 int
 pr_CreateUser(prname name, afs_int32 *id)
 {
     afs_int32 code;
 
+    code = check_length(name);
+    if (code)
+       return code;
     stolower(name);
     if (*id) {
        code = ubik_PR_INewEntry(pruclient, 0, name, *id, 0);
-       return code;
     } else {
        code = ubik_PR_NewEntry(pruclient, 0, name, 0, 0, id);
-       return code;
     }
-
+    return code;
 }
 
 int
@@ -357,6 +375,10 @@ pr_CreateGroup(prname name, prname owner, afs_int32 *id)
     afs_int32 oid = 0;
     afs_int32 flags = 0;
 
+    code = check_length(name);
+    if (code)
+       return code;
+    /* pr_SNameToId will check owner's length. */
     stolower(name);
     if (owner) {
        code = pr_SNameToId(owner, &oid);
@@ -368,20 +390,19 @@ pr_CreateGroup(prname name, prname owner, afs_int32 *id)
     flags |= PRGRP;
     if (*id) {
        code = ubik_PR_INewEntry(pruclient, 0, name, *id, oid);
-       return code;
     } else {
        code = ubik_PR_NewEntry(pruclient, 0, name, flags, oid, id);
-       return code;
     }
+    return code;
 }
 
 int
-pr_Delete(char *name)
+pr_Delete(prname name)
 {
     afs_int32 code;
     afs_int32 id;
 
-    stolower(name);
+    /* pr_SNameToId both checks the length of name and lowercases it. */
     code = pr_SNameToId(name, &id);
     if (code)
        return code;
@@ -401,12 +422,18 @@ pr_DeleteByID(afs_int32 id)
 }
 
 int
-pr_AddToGroup(char *user, char *group)
+pr_AddToGroup(prname user, prname group)
 {
     afs_int32 code;
     namelist lnames;
     idlist lids;
 
+    code = check_length(user);
+    if (code)
+       return code;
+    code = check_length(group);
+    if (code)
+       return code;
     lnames.namelist_len = 2;
     lnames.namelist_val = malloc(2 * PR_MAXNAMELEN);
     strncpy(lnames.namelist_val[0], user, PR_MAXNAMELEN);
@@ -417,6 +444,10 @@ pr_AddToGroup(char *user, char *group)
     if (code)
        goto done;
     /* if here, still could be missing an entry */
+    if (lids.idlist_len != 2) {
+       code = PRINTERNAL;
+       goto done;
+    }
     if (lids.idlist_val[0] == ANONYMOUSID
        || lids.idlist_val[1] == ANONYMOUSID) {
        code = PRNOENT;
@@ -434,12 +465,18 @@ pr_AddToGroup(char *user, char *group)
 }
 
 int
-pr_RemoveUserFromGroup(char *user, char *group)
+pr_RemoveUserFromGroup(prname user, prname group)
 {
     afs_int32 code;
     namelist lnames;
     idlist lids;
 
+    code = check_length(user);
+    if (code)
+       return code;
+    code = check_length(group);
+    if (code)
+       return code;
     lnames.namelist_len = 2;
     lnames.namelist_val = malloc(2 * PR_MAXNAMELEN);
     strncpy(lnames.namelist_val[0], user, PR_MAXNAMELEN);
@@ -450,6 +487,10 @@ pr_RemoveUserFromGroup(char *user, char *group)
     if (code)
        goto done;
 
+    if (lids.idlist_len != 2) {
+       code = PRINTERNAL;
+       goto done;
+    }
     if (lids.idlist_val[0] == ANONYMOUSID
        || lids.idlist_val[1] == ANONYMOUSID) {
        code = PRNOENT;
@@ -473,8 +514,12 @@ pr_NameToId(namelist *names, idlist *ids)
     afs_int32 code;
     afs_int32 i;
 
-    for (i = 0; i < names->namelist_len; i++)
+    for (i = 0; i < names->namelist_len; i++) {
+       code = check_length(names->namelist_val[i]);
+       if (code)
+           return code;
        stolower(names->namelist_val[i]);
+    }
     code = ubik_PR_NameToID(pruclient, 0, names, ids);
     return code;
 }
@@ -486,6 +531,9 @@ pr_SNameToId(prname name, afs_int32 *id)
     idlist lids;
     afs_int32 code;
 
+    code = check_length(name);
+    if (code)
+       return code;
     lids.idlist_len = 0;
     lids.idlist_val = 0;
     lnames.namelist_len = 1;
@@ -504,15 +552,35 @@ pr_SNameToId(prname name, afs_int32 *id)
     return code;
 }
 
+/*
+ * Like ubik_PR_IDToName, but enforces that the output prnames are
+ * interpretable as C strings (i.e., NUL-terminated).
+ */
 int
-pr_IdToName(idlist *ids, namelist *names)
+string_PR_IDToName(struct ubik_client *client, afs_int32 flags,
+                  idlist *ids, namelist *names)
 {
     afs_int32 code;
+    int i;
 
-    code = ubik_PR_IDToName(pruclient, 0, ids, names);
+    code = ubik_PR_IDToName(client, flags, ids, names);
+    if (code)
+       return code;
+    for (i = 0; i < names->namelist_len; i++) {
+       code = check_length(names->namelist_val[i]);
+       if (code)
+           return code;
+    }
     return code;
 }
 
+
+int
+pr_IdToName(idlist *ids, namelist *names)
+{
+    return string_PR_IDToName(pruclient, 0, ids, names);
+}
+
 int
 pr_SIdToName(afs_int32 id, prname name)
 {
@@ -525,8 +593,7 @@ pr_SIdToName(afs_int32 id, prname name)
     *lids.idlist_val = id;
     lnames.namelist_len = 0;
     lnames.namelist_val = 0;
-    code = ubik_PR_IDToName(pruclient, 0, &lids, &lnames);
-
+    code = pr_IdToName(&lids, &lnames);
     if (lnames.namelist_val)
        strncpy(name, lnames.namelist_val[0], PR_MAXNAMELEN);
     else if (code == 0)
@@ -599,19 +666,28 @@ pr_GetHostCPS(afs_uint32 host, prlist *CPS)
 }
 
 int
-pr_ListMembers(char *group, namelist *lnames)
+pr_ListMembers(prname group, namelist *lnames)
 {
     afs_int32 code;
     afs_int32 gid;
+    int i;
 
     memset(lnames, 0, sizeof(namelist));
 
+    /* pr_SNameToId checks the length of group. */
     code = pr_SNameToId(group, &gid);
     if (code)
        return code;
     if (gid == ANONYMOUSID)
        return PRNOENT;
     code = pr_IDListMembers(gid, lnames);
+    if (code)
+       return code;
+    for (i = 0; i < lnames->namelist_len; i++) {
+       code = check_length(lnames->namelist_val[i]);
+       if (code)
+           return code;
+    }
     return code;
 }
 
@@ -773,13 +849,16 @@ pr_ListEntry(afs_int32 id, struct prcheckentry *aentry)
     afs_int32 code;
 
     code = ubik_PR_ListEntry(pruclient, 0, id, aentry);
-    return code;
+    if (code)
+       return code;
+    return check_length(aentry->name);
 }
 
 afs_int32
 pr_ListEntries(int flag, afs_int32 startindex, afs_int32 *nentries, struct prlistentries **entries, afs_int32 *nextstartindex)
 {
     afs_int32 code;
+    int i;
     prentries bulkentries;
 
     *nentries = 0;
@@ -791,18 +870,33 @@ pr_ListEntries(int flag, afs_int32 startindex, afs_int32 *nentries, struct prlis
     code =
        ubik_PR_ListEntries(pruclient, 0, flag, startindex,
                  &bulkentries, nextstartindex);
-    *nentries = bulkentries.prentries_len;
-    *entries = bulkentries.prentries_val;
+    if (code)
+       return code;
+    for (i = 0; i < bulkentries.prentries_len; i++) {
+       /* XXX should we try to return all the other entries? */
+       code = check_length(bulkentries.prentries_val[i].name);
+       if (code)
+           goto out;
+    }
+
+out:
+    if (code != 0) {
+       xdr_free((xdrproc_t)xdr_prentries, &bulkentries);
+    } else {
+       *nentries = bulkentries.prentries_len;
+       *entries = bulkentries.prentries_val;
+    }
     return code;
 }
 
 int
-pr_CheckEntryByName(char *name, afs_int32 *id, char *owner, char *creator)
+pr_CheckEntryByName(prname name, afs_int32 *id, prname owner, prname creator)
 {
     /* struct prcheckentry returns other things, which aren't useful to show at this time. */
     afs_int32 code;
     struct prcheckentry aentry;
 
+    /* pr_SNameToId will check name's length. */
     code = pr_SNameToId(name, id);
     if (code)
        return code;
@@ -822,12 +916,13 @@ pr_CheckEntryByName(char *name, afs_int32 *id, char *owner, char *creator)
 }
 
 int
-pr_CheckEntryById(char *name, afs_int32 id, char *owner, char *creator)
+pr_CheckEntryById(prname name, afs_int32 id, prname owner, prname creator)
 {
     /* struct prcheckentry returns other things, which aren't useful to show at this time. */
     afs_int32 code;
     struct prcheckentry aentry;
 
+    /* XXX ListEntry RPC gives us the name back so should avoid extra RPC */
     code = pr_SIdToName(id, name);
     if (code)
        return code;
@@ -847,12 +942,13 @@ pr_CheckEntryById(char *name, afs_int32 id, char *owner, char *creator)
 }
 
 int
-pr_ChangeEntry(char *oldname, char *newname, afs_int32 *newid, char *newowner)
+pr_ChangeEntry(prname oldname, prname newname, afs_int32 *newid, prname newowner)
 {
     afs_int32 code;
     afs_int32 id;
     afs_int32 oid = 0;
 
+    /* pr_SNameToId takes care of length checks for us. */
     code = pr_SNameToId(oldname, &id);
     if (code)
        return code;
@@ -873,12 +969,18 @@ pr_ChangeEntry(char *oldname, char *newname, afs_int32 *newid, char *newowner)
 }
 
 int
-pr_IsAMemberOf(char *uname, char *gname, afs_int32 *flag)
+pr_IsAMemberOf(prname uname, prname gname, afs_int32 *flag)
 {
     afs_int32 code;
     namelist lnames;
     idlist lids;
 
+    code = check_length(uname);
+    if (code)
+       return code;
+    code = check_length(gname);
+    if (code)
+       return code;
     stolower(uname);
     stolower(gname);
     lnames.namelist_len = 2;
@@ -894,6 +996,11 @@ pr_IsAMemberOf(char *uname, char *gname, afs_int32 *flag)
        xdr_free((xdrproc_t) xdr_idlist, &lids);
        return code;
     }
+    if (lids.idlist_len != 2) {
+       free(lnames.namelist_val);
+       xdr_free((xdrproc_t) xdr_idlist, &lids);
+       return PRINTERNAL;
+    }
     code =
        ubik_PR_IsAMemberOf(pruclient, 0, lids.idlist_val[0],
                  lids.idlist_val[1], flag);