Don't trust # of entries from ListAttributes

ListAttributes returns the number of entries in its array as an RPC
argument. But, we can't trust this, as it could be manipulated and
end up pointing past the end of the returned array (which is counted,
so the entries argument is actually pointless).

Add bounds checking to the functions which use this value to prevent
this problem.

(cherry picked from commit 8992210f27671673a89a541776aa105238ad14cf)
Reviewed-on: http://gerrit.openafs.org/3597
Reviewed-by: Derrick Brashear <shadow@dementia.org>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Change-Id: I318f2f956a48f10e91590ad9f28fab868d8ceb60
Reviewed-on: http://gerrit.openafs.org/4134
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementia.org>

diff --git a/src/bucoord/commands.c b/src/bucoord/commands.c
index 9b2570715f05c0525f48396930244f89bf5d5375..ac9dcbb01f00160f9735d4631c25644fb0706a2b 100644 (file)
--- a/src/bucoord/commands.c
+++ b/src/bucoord/commands.c
@@ -258,6 +258,11 @@ EvalVolumeSet2(struct bc_config *aconfig,
            if (nentries == 0)
                ERROR(RXGEN_OPCODE);    /* Use EvalVolumeSet1 */
 
+           if (nentries < 0)
+               nentries = 0;
+           if (nentries > bulkentries.nbulkentries_len)
+               nentries = bulkentries.nbulkentries_len;
+
            /* Step through each entry and add it to the list of volumes */
            entries = bulkentries.nbulkentries_val;
            for (e = 0; e < nentries; e++) {

diff --git a/src/libadmin/vos/vosutils.c b/src/libadmin/vos/vosutils.c
index aeb493fc42fd8bbbcd07527cd923fb2ecea57727..4672651c873bb37e6971294db6e38edb19d09772 100644 (file)
--- a/src/libadmin/vos/vosutils.c
+++ b/src/libadmin/vos/vosutils.c
@@ -258,6 +258,10 @@ VLDB_ListAttributes(afs_cell_handle_p cellHandle,
                    cellHandle->vos_new = 0;
                }
            } else {
+               if (*entriesp < 0)
+                   *entriesp = 0;
+               if (*entriesp > blkentriesp->nbulkentries_len)
+                   *entriesp = blkentriesp->nbulkentries_len;
                rc = 1;
            }
        } else {
@@ -266,6 +270,12 @@ VLDB_ListAttributes(afs_cell_handle_p cellHandle,
                ubik_VL_ListAttributes(cellHandle->vos, 0, attrp,
                          entriesp, &arrayEntries);
            if (tst == 0) {
+
+               if (*entriesp < 0)
+                   *entriesp = 0;
+               if (*entriesp > arrayEntries.bulkentries_len)
+                   *entriesp = arrayEntries.bulkentries_len;
+
                blkentriesp->nbulkentries_val =
                    (nvldbentry *) malloc(*entriesp * sizeof(*blkentriesp));
                if (blkentriesp->nbulkentries_val != NULL) {
@@ -281,6 +291,7 @@ VLDB_ListAttributes(afs_cell_handle_p cellHandle,
                if (arrayEntries.bulkentries_val) {
                    free(arrayEntries.bulkentries_val);
                }
+
                rc = 1;
            }
        }

diff --git a/src/volser/vsutils.c b/src/volser/vsutils.c
index b89e2e05df09acdaa1a205432f2e2164709a809b..711673f63311752ec3902dce44a1414c62587da2 100644 (file)
--- a/src/volser/vsutils.c
+++ b/src/volser/vsutils.c
@@ -249,6 +249,12 @@ VLDB_ListAttributes(VldbListByAttributes *attrp,
        if (code)
            return code;
 
+       /* Ensure the number of entries claimed matches the no. returned */
+       if (*entriesp < 0)
+           *entriesp = 0;
+       if (*entriesp > arrayEntries.bulkentries_len)
+           *entriesp = arrayEntries.bulkentries_len;
+
        convertBulkToNBulk(&arrayEntries, blkentriesp);
 
        xdr_free((xdrproc_t) xdr_bulkentries, &arrayEntries);
@@ -264,6 +270,16 @@ VLDB_ListAttributes(VldbListByAttributes *attrp,
            newvlserver = vltype_new;
        }
     }
+
+    if (code)
+       return code;
+
+    /* Ensure the number of entries claimed matches the no. returned */
+    if (*entriesp < 0)
+       *entriesp = 0;
+    if (*entriesp > blkentriesp->nbulkentries_len)
+       *entriesp = blkentriesp->nbulkentries_len;
+
     return code;
 }
 
@@ -281,6 +297,14 @@ VLDB_ListAttributesN2(VldbListByAttributes *attrp,
         code =
             ubik_VL_ListAttributesN2(cstruct, 0, attrp, (name ? name : ""),
                                      thisindex, nentriesp, blkentriesp, nextindexp);
+       if (code)
+           return code;
+
+       /* Ensure the number of entries claimed matches the no. returned */
+       if (*nentriesp < 0)
+           *nentriesp = 0;
+       if (*nentriesp > blkentriesp->nbulkentries_len)
+           *nentriesp = blkentriesp->nbulkentries_len;
     }
     return code;
 }