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.

Change-Id: I62398d8f6b5c54318c1a42f1bad67a21c90ef944
Reviewed-on: http://gerrit.openafs.org/3597
Reviewed-by: Derrick Brashear <shadow@dementia.org>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

diff --git a/src/bucoord/commands.c b/src/bucoord/commands.c
index 324510d65cb4c8f8ad15c5b22d016d884e9e0a7f..5e4ba53d65150995a5fe978d550d5cc26ad21a09 100644 (file)
--- a/src/bucoord/commands.c
+++ b/src/bucoord/commands.c
@@ -259,6 +259,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 c2325bc810a3181c0569bd2da163d977d3a8c81e..9f18ab6def158636d44b46b295cc8ab45b9762e0 100644 (file)
--- a/src/libadmin/vos/vosutils.c
+++ b/src/libadmin/vos/vosutils.c
@@ -259,6 +259,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 {
@@ -267,6 +271,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) {
@@ -282,6 +292,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 447d39a117d0c6276e60631a641c9757b4dccd9b..33ecefce6aa297a26ca31f697bbb0a47ff2325b7 100644 (file)
--- a/src/volser/vsutils.c
+++ b/src/volser/vsutils.c
@@ -484,6 +484,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);
@@ -496,6 +502,16 @@ VLDB_ListAttributes(VldbListByAttributes *attrp,
         newvlserver = vltype_old;      /* Doesn't support new interface */
         goto tryold;
     }
+
+    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;
 }
 
@@ -517,6 +533,12 @@ VLDB_ListAttributesU(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;
+
        convertBulkToUBulk(&arrayEntries, blkentriesp);
 
        xdr_free((xdrproc_t) xdr_bulkentries, &arrayEntries);
@@ -530,10 +552,15 @@ VLDB_ListAttributesU(VldbListByAttributes *attrp,
         newvlserver = vltype_old;      /* Doesn't support new interface */
         goto tryold;
     }
-
     if (code)
        return code;
 
+    /* Ensure the number of entries claimed matches the no. returned */
+    if (*entriesp < 0)
+       *entriesp = 0;
+    if (*entriesp > narrayEntries.nbulkentries_len)
+       *entriesp = narrayEntries.nbulkentries_len;
+
     convertNBulkToUBulk(&narrayEntries, blkentriesp);
 
     xdr_free((xdrproc_t) xdr_bulkentries, &narrayEntries);
@@ -554,6 +581,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;
 }
@@ -578,6 +613,12 @@ VLDB_ListAttributesN2U(VldbListByAttributes *attrp,
         if (code)
            return code;
 
+       /* Ensure the number of entries claimed matches the no. returned */
+       if (*nentriesp < 0)
+           *nentriesp = 0;
+       if (*nentriesp > narrayEntries.nbulkentries_len)
+           *nentriesp = narrayEntries.nbulkentries_len;
+
        convertNBulkToUBulk(&narrayEntries, blkentriesp);
 
        xdr_free((xdrproc_t) xdr_bulkentries, &narrayEntries);