From 511fc5d4b50bce273c16acef7b551b8517a6bf46 Mon Sep 17 00:00:00 2001 From: Simon Wilkinson Date: Sun, 26 Dec 2010 14:54:43 +0000 Subject: [PATCH] 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 Tested-by: BuildBot Change-Id: I318f2f956a48f10e91590ad9f28fab868d8ceb60 Reviewed-on: http://gerrit.openafs.org/4134 Tested-by: BuildBot Reviewed-by: Derrick Brashear --- src/bucoord/commands.c | 5 +++++ src/libadmin/vos/vosutils.c | 11 +++++++++++ src/volser/vsutils.c | 24 ++++++++++++++++++++++++ 3 files changed, 40 insertions(+) diff --git a/src/bucoord/commands.c b/src/bucoord/commands.c index 9b2570715..ac9dcbb01 100644 --- 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 aeb493fc4..4672651c8 100644 --- 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 b89e2e05d..711673f63 100644 --- 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; } -- 2.39.5