jimzah/openafs
1
0
Fork 0
mirror of https://git.openafs.org/openafs.git synced 2025-01-18 15:00:12 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

OPENAFS-SA-2024-003: Check sanity on lengths of RPC returned arrays

Browse Source 
CVE-2024-10397

Various RPCs return a variable-length array in an OUT argument, but
are only supposed to return specific sizes. A few instances of this
include the following (but this is not an exhaustive list):

- AFSVolListOneVolume should only return a single volintInfo.

- PR_NameToID should return the same number of IDs as names given.

- VL_GetAddrsU should return the same number of addresses as the
  'nentries' OUT argument.

Some callers of these RPCs just assume that the server has not
violated these rules. If the server responds with a nonsensical array
size, this could cause us to read beyond the end of the array, or
cause a NULL dereference or other errors.

For example, some callers of VL_GetAddrsU will iterate over 'nentries'
addresses, even if the 'blkaddrs' OUT argument contains fewer entries.
Or with AFSVolListOneVolume, some callers assume that at least 1
volintInfo has been returned; if 0 have been returned, we can try to
access a NULL array.

To avoid all of this, add various sanity checks on the relevant
returned lengths of these RPCs. For most cases, if the lengths are not
sane, return an internal error from the appropriate subsystem (or
RXGEN_CC_UNMARSHAL if there isn't one). For VL_GetAddrsU, if
'nentries' is too long, just set it to the length of the returned
array.

FIXES 135043

Change-Id: Ibdc7837ab09b4765436fc4c0d780e695bba07128
Reviewed-on: https://gerrit.openafs.org/15921
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
This commit is contained in:
Andrew Deason 2020-10-15 23:18:53 -05:00 committed by Benjamin Kaduk
parent 13413eceed
commit c732715e4e
17 changed files with 151 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/WINNT/afsd/cm_getaddrs.c
View File

@ -260,6 +260,8 @@ cm_GetAddrsU(cm_cell_t *cellp, cm_user_t *userp, cm_req_t *reqp,
if (code)
return CM_ERROR_RETRY;
nentries = min(nentries, addrs.bulkaddrs_len);
if (nentries == 0) {
xdr_free((xdrproc_t) xdr_bulkaddrs, &addrs);
code = CM_ERROR_INVAL;

7
src/WINNT/afsd/cm_vnodeops.c
View File

@ -2519,6 +2519,13 @@ cm_TryBulkStatRPC(cm_scache_t *dscp, cm_bulkStat_t *bbp, cm_user_t *userp, cm_re
}
rx_PutConnection(rxconnp);
if (code == 0) {
if (statStruct.AFSBulkStats_len != filesThisCall ||
callbackStruct.AFSCBs_len != filesThisCall) {
code = RXGEN_CC_UNMARSHAL;
}
}
/*
* If InlineBulk RPC was called and it succeeded,
* then pull out the return code from the status info

4
src/afs/afs_volume.c
View File

@ -1228,6 +1228,10 @@ LockAndInstallUVolumeEntry(struct volume *av, struct uvldbentry *ve, int acell,
return;
}
if (addrs.bulkaddrs_len < nentries) {
nentries = addrs.bulkaddrs_len;
}
addrp = addrs.bulkaddrs_val;
for (k = 0; k < nentries; k++) {
addrp[k] = htonl(addrp[k]);

6
src/bucoord/ubik_db_if.c
View File

@ -116,6 +116,12 @@ afs_int32 bcdb_listDumps (afs_int32 sflags, afs_int32 groupId,
free(dumpsList.budb_dumpsList_val);
if (flagsList.budb_dumpsList_val)
free(flagsList.budb_dumpsList_val);
if (code == 0 &&
dumpsPtr->budb_dumpsList_len != flagsPtr->budb_dumpsList_len) {
code = BUDB_INTERNALERROR;
}
return (code);
}

18
src/butc/dump.c
View File

@ -173,6 +173,18 @@ Bind(afs_uint32 server)
return (curr_fromconn);
}
static int
ListOneVolume(struct rx_connection *aconn, afs_int32 apart, afs_uint32 avolid,
volEntries *entries)
{
afs_int32 code;
code = AFSVolListOneVolume(aconn, apart, avolid, entries);
if (code == 0 && entries->volEntries_len != 1) {
code = VOLSERFAILEDOP;
}
return code;
}
/* notes
* 1) save the chunksize or otherwise ensure tape space remaining is
* check frequently enough
@ -217,8 +229,7 @@ dumpVolume(struct tc_dumpDesc * curDump, struct dumpRock * dparamsPtr)
/* Determine when the volume was last cloned and updated */
volumeInfo.volEntries_val = (volintInfo *) 0;
volumeInfo.volEntries_len = 0;
rc = AFSVolListOneVolume(fromconn, curDump->partition, curDump->vid,
&volumeInfo);
rc = ListOneVolume(fromconn, curDump->partition, curDump->vid, &volumeInfo);
if (rc)
ERROR_EXIT(rc);
updatedate = volumeInfo.volEntries_val[0].updateDate;
@ -543,8 +554,7 @@ xbsaDumpVolume(struct tc_dumpDesc * curDump, struct dumpRock * dparamsPtr)
/* Determine when the volume was last cloned and updated */
volumeInfo.volEntries_val = (volintInfo *) 0;
volumeInfo.volEntries_len = 0;
rc = AFSVolListOneVolume(fromconn, curDump->partition, curDump->vid,
&volumeInfo);
rc = ListOneVolume(fromconn, curDump->partition, curDump->vid, &volumeInfo);
if (rc)
ERROR_EXIT(rc);
updatedate = volumeInfo.volEntries_val[0].updateDate;

12
src/libadmin/adminutil/afs_utilAdmin.c
View File

@ -2324,6 +2324,9 @@ util_CMClientConfig(struct rx_connection *conn, afs_ClientConfig_p config,
afs_uint32 allocbytes;
struct cacheConfig tconfig;
tconfig.cacheConfig_val = NULL;
tconfig.cacheConfig_len = 0;
if (conn == NULL) {
tst = ADMRXCONNNULL;
goto fail_util_CMClientConfig;
@ -2335,8 +2338,6 @@ util_CMClientConfig(struct rx_connection *conn, afs_ClientConfig_p config,
}
config->clientVersion = AFS_CLIENT_RETRIEVAL_VERSION;
tconfig.cacheConfig_val = NULL;
tconfig.cacheConfig_len = 0;
tst =
RXAFSCB_GetCacheConfig(conn, config->clientVersion,
&config->serverVersion, &allocbytes, &tconfig);
@ -2345,12 +2346,17 @@ util_CMClientConfig(struct rx_connection *conn, afs_ClientConfig_p config,
goto fail_util_CMClientConfig;
}
if (tconfig.cacheConfig_len != sizeof(cm_initparams_v1)/sizeof(afs_uint32)) {
tst = RXGEN_CC_UNMARSHAL;
goto fail_util_CMClientConfig;
}
UnmarshallCMClientConfig(config->serverVersion, tconfig.cacheConfig_val,
&config->c);
rc = 1;
free(tconfig.cacheConfig_val);
fail_util_CMClientConfig:
free(tconfig.cacheConfig_val);
if (st != NULL) {
*st = tst;

4
src/libadmin/pts/afs_ptsAdmin.c
View File

@ -125,6 +125,10 @@ TranslatePTSNames(const afs_cell_handle_p cellHandle, namelist * names,
goto fail_TranslatePTSNames;
}
if (ids->idlist_len != names->namelist_len) {
tst = ADMPTSFAILEDNAMETRANSLATE;
goto fail_TranslatePTSNames;
}
/*
* Check to see if the lookup failed

8
src/libadmin/vos/afs_vosAdmin.c
View File

@ -1217,6 +1217,10 @@ GetServerRPC(void *rpc_specific, int slot, int *last_item,
goto fail_GetServerRPC;
}
if (addr_multi.bulkaddrs_len < total_multi) {
total_multi = addr_multi.bulkaddrs_len;
}
/*
* Remove any bogus IP addresses which the user may have
* been unable to remove.
@ -1368,6 +1372,10 @@ vos_FileServerGetBegin(const void *cellHandle, vos_MessageCallBack_t callBack,
goto fail_vos_FileServerGetBegin;
}
if (serv->addresses.bulkaddrs_len < serv->total_addresses) {
serv->total_addresses = serv->addresses.bulkaddrs_len;
}
/*
* Remove any bogus IP addresses which the user may have
* been unable to remove.

6
src/libadmin/vos/vosutils.c
View File

@ -324,6 +324,9 @@ VLDB_ListAttributesN2(afs_cell_handle_p cellHandle,
nextindexp);
if (!tst) {
rc = 1;
if (blkentriesp->nbulkentries_len < *nentriesp) {
*nentriesp = blkentriesp->nbulkentries_len;
}
}
if (st != NULL) {
@ -365,6 +368,9 @@ VLDB_IsSameAddrs(afs_cell_handle_p cellHandle, afs_int32 serv1,
*equal = 0;
goto fail_VLDB_IsSameAddrs;
}
if (addrs.bulkaddrs_len < nentries) {
nentries = addrs.bulkaddrs_len;
}
addrp = addrs.bulkaddrs_val;
for (i = 0; i < nentries; i++, addrp++) {

22
src/libadmin/vos/vsprocs.c
View File

@ -600,7 +600,7 @@ UV_MoveVolume(afs_cell_handle_p cellHandle, afs_uint32 afromvol,
*/
volumeInfo.volEntries_val = (volintInfo *) 0; /*this hints the stub to allocate space */
volumeInfo.volEntries_len = 0;
tst = AFSVolListOneVolume(fromconn, afrompart, afromvol, &volumeInfo);
tst = ListOneVolume(fromconn, afrompart, afromvol, &volumeInfo);
if (tst) {
goto fail_UV_MoveVolume;
}
@ -1162,6 +1162,18 @@ UV_BackupVolume(afs_cell_handle_p cellHandle, afs_int32 aserver,
return rc;
}
static int
ListOneVolume(struct rx_connection *aconn, afs_int32 apart, afs_uint32 avolid,
volEntries *entries)
{
afs_int32 code;
code = AFSVolListOneVolume(aconn, apart, avolid, entries);
if (code == 0 && entries->volEntries_len != 1) {
code = VOLSERFAILEDOP;
}
return code;
}
static int
DelVol(struct rx_connection *conn, afs_uint32 vid, afs_int32 part,
afs_int32 flags)
@ -1314,7 +1326,7 @@ VolumeExists(afs_cell_handle_p cellHandle, afs_int32 server,
if (conn) {
volumeInfo.volEntries_val = (volintInfo *) 0;
volumeInfo.volEntries_len = 0;
tst = AFSVolListOneVolume(conn, partition, volumeid, &volumeInfo);
tst = ListOneVolume(conn, partition, volumeid, &volumeInfo);
if (volumeInfo.volEntries_val)
free(volumeInfo.volEntries_val);
if (tst == VOLSERILLEGAL_PARTITION) {
@ -2773,7 +2785,9 @@ UV_XListOneVolume(struct rx_connection *server, afs_int32 a_partID,
volumeXInfo.volXEntries_len = 0;
tst = AFSVolXListOneVolume(server, a_partID, a_volID, &volumeXInfo);
if (tst == 0 && volumeXInfo.volXEntries_len != 1) {
tst = VOLSERFAILEDOP;
}
if (tst) {
goto fail_UV_XListOneVolume;
} else {
@ -2833,7 +2847,7 @@ int UV_ListOneVolume(struct rx_connection *server, afs_int32 a_partID,
volumeInfo.volEntries_val = (volintInfo *) 0;
volumeInfo.volEntries_len = 0;
tst = AFSVolListOneVolume(server, a_partID, a_volID, &volumeInfo);
tst = ListOneVolume(server, a_partID, a_volID, &volumeInfo);
if (tst) {
goto fail_UV_ListOneVolume;

7
src/libafscp/afscp_server.c
View File

@ -305,6 +305,9 @@ afscp_ServerById(struct afscp_cell *thecell, afsUUID * u)
if (code != 0) {
return NULL;
}
if (addrs.bulkaddrs_len < nentries) {
nentries = addrs.bulkaddrs_len;
}
if (nentries > AFS_MAXHOSTS) {
nentries = AFS_MAXHOSTS;
/* XXX I don't want to do *that* much dynamic allocation */
@ -405,6 +408,10 @@ afscp_ServerByAddr(struct afscp_cell *thecell, afs_uint32 addr)
afsUUID_to_string(&uuid, &s);
afs_dprintf(("GetServerByAddr 0x%x -> uuid %s\n", addr, s.buffer));
if (addrs.bulkaddrs_len < nentries) {
nentries = addrs.bulkaddrs_len;
}
if (nentries > AFS_MAXHOSTS) {
nentries = AFS_MAXHOSTS;
/* XXX I don't want to do *that* much dynamic allocation */

6
src/ptserver/ptuser.c
View File

@ -551,6 +551,9 @@ pr_NameToId(namelist *names, idlist *ids)
stolower(names->namelist_val[i]);
}
code = ubik_PR_NameToID(pruclient, 0, names, ids);
if (code == 0 && ids->idlist_len != names->namelist_len) {
code = PRINTERNAL;
}
return code;
}
@ -596,6 +599,9 @@ string_PR_IDToName(struct ubik_client *client, afs_int32 flags,
code = ubik_PR_IDToName(client, flags, ids, names);
if (code)
return code;
if (names->namelist_len != ids->idlist_len) {
return PRINTERNAL;
}
for (i = 0; i < names->namelist_len; i++) {
code = check_length(names->namelist_val[i]);
if (code)

4
src/venus/cacheout.c
View File

@ -71,6 +71,10 @@ ListServers(void)
return 1;
}
if (addrs.bulkaddrs_len < server_count) {
server_count = addrs.bulkaddrs_len;
}
for (i = 0; i < server_count; ++i) {
ip = addrs.bulkaddrs_val[i];

4
src/viced/host.c
View File

@ -30,6 +30,7 @@
#include <afs/ihandle.h>
#include <afs/acl.h>
#include <afs/ptclient.h>
#include <afs/pterror.h>
#include <afs/ptuser.h>
#include <afs/prs_fs.h>
#include <afs/auth.h>
@ -403,6 +404,9 @@ hpr_NameToId(namelist *names, idlist *ids)
for (i = 0; i < names->namelist_len; i++)
stolower(names->namelist_val[i]);
code = ubik_PR_NameToID(uclient, 0, names, ids);
if (code == 0 && ids->idlist_len != names->namelist_len) {
code = PRINTERNAL;
}
return code;
}

21
src/vlserver/vlclient.c
View File

@ -561,6 +561,9 @@ handleit(struct cmd_syndesc *as, void *arock)
printf("VL_ListAttributes returned code = %d\n", code);
continue;
}
if (entries.bulkentries_len < nentries) {
nentries = entries.bulkentries_len;
}
entry = (struct vldbentry *)entries.bulkentries_val;
for (i = 0; i < nentries; i++, entry++)
display_entry(entry, 0);
@ -596,6 +599,9 @@ handleit(struct cmd_syndesc *as, void *arock)
code);
break;
}
if (entries.nbulkentries_len < nentries) {
nentries = entries.nbulkentries_len;
}
t += nentries;
entry = (struct nvldbentry *)entries.nbulkentries_val;
@ -764,6 +770,9 @@ handleit(struct cmd_syndesc *as, void *arock)
printf("VL_GetAddrs returned code = %d\n", code);
continue;
}
if (addrs.bulkaddrs_len < nentries) {
nentries = addrs.bulkaddrs_len;
}
addrp = addrs.bulkaddrs_val;
for (i = 0; i < nentries; i++, addrp++) {
if ((*addrp & 0xff000000) == 0xff000000)
@ -789,6 +798,9 @@ handleit(struct cmd_syndesc *as, void *arock)
printf("VL_GetAddrs returned code = %d\n", code);
continue;
}
if (addrs.bulkaddrs_len < nentries) {
nentries = addrs.bulkaddrs_len;
}
addrp = addrs.bulkaddrs_val;
for (i = 0; i < nentries; i++, addrp++) {
if ((*addrp & 0xff000000) == 0xff000000) {
@ -814,6 +826,9 @@ handleit(struct cmd_syndesc *as, void *arock)
printf("VL_GetAddrsU returned code = %d\n", code);
continue;
}
if (mhaddrs.bulkaddrs_len < mhnentries) {
mhnentries = mhaddrs.bulkaddrs_len;
}
printf
(" [%d]: uuid[%x,%x,%x,%x,%x,%x,%x,%x,%x,%x,%x]\n addrunique=%d, ip address(es):\n",
attrs.index, uuid.time_low, uuid.time_mid,
@ -864,6 +879,9 @@ handleit(struct cmd_syndesc *as, void *arock)
printf("VL_GetAddrs returned code = %d\n", code);
continue;
}
if (addrs1.bulkaddrs_len < nentries1) {
nentries1 = addrs1.bulkaddrs_len;
}
addrp1 = addrs1.bulkaddrs_val;
for (i = 0; i < nentries1; i++, addrp1++) {
if ((*addrp1 & 0xff000000) != 0xff000000) {
@ -887,6 +905,9 @@ handleit(struct cmd_syndesc *as, void *arock)
printf("VL_GetAddrsU returned code = %d\n", code);
break;
}
if (addrs2.bulkaddrs_len < nentries2) {
nentries2 = addrs2.bulkaddrs_len;
}
addrp2 = addrs2.bulkaddrs_val;
for (j = 0; j < nentries2; j++) {

42
src/volser/vsprocs.c
View File

@ -2850,6 +2850,18 @@ UV_BackupVolume(afs_uint32 aserver, afs_int32 apart, afs_uint32 avolid)
return error;
}
static int
ListOneVolume(struct rx_connection *aconn, afs_int32 apart, afs_uint32 avolid,
volEntries *entries)
{
afs_int32 code;
code = AFSVolListOneVolume(aconn, apart, avolid, entries);
if (code == 0 && entries->volEntries_len != 1) {
code = VOLSERFAILEDOP;
}
return code;
}
/* Make a new clone of volume <avolid> on <aserver> and <apart>
* using volume ID <acloneid>, or a new ID allocated from the VLDB.
* The new volume is named by <aname>, or by appending ".clone" to
@ -2877,7 +2889,7 @@ UV_CloneVolume(afs_uint32 aserver, afs_int32 apart, afs_uint32 avolid,
if (!aname) {
volumeInfo.volEntries_val = (volintInfo *) 0;
volumeInfo.volEntries_len = 0;
code = AFSVolListOneVolume(aconn, apart, avolid, &volumeInfo);
code = ListOneVolume(aconn, apart, avolid, &volumeInfo);
if (code) {
fprintf(stderr, "Could not get info for volume %lu\n",
(unsigned long)avolid);
@ -3598,9 +3610,8 @@ UV_ReleaseVolume(afs_uint32 afromvol, afs_uint32 afromserver,
}
volumeInfo.volEntries_val = NULL;
volumeInfo.volEntries_len = 0;
code = AFSVolListOneVolume(conn, entry.serverPartition[vldbindex],
entry.volumeId[ROVOL],
&volumeInfo);
code = ListOneVolume(conn, entry.serverPartition[vldbindex],
entry.volumeId[ROVOL], &volumeInfo);
if (code) {
fprintf(STDERR, "Could not fetch information about RO vol %lu from server %s\n",
(unsigned long)entry.volumeId[ROVOL],
@ -3628,8 +3639,7 @@ UV_ReleaseVolume(afs_uint32 afromvol, afs_uint32 afromserver,
volEntries volumeInfo;
volumeInfo.volEntries_val = NULL;
volumeInfo.volEntries_len = 0;
code = AFSVolListOneVolume(fromconn, afrompart, afromvol,
&volumeInfo);
code = ListOneVolume(fromconn, afrompart, afromvol, &volumeInfo);
if (code) {
fprintf(STDERR, "Could not fetch information about RW vol %lu from server %s\n",
(unsigned long)afromvol,
@ -3971,6 +3981,10 @@ UV_ReleaseVolume(afs_uint32 afromvol, afs_uint32 afromserver,
nservers = 1;
}
if (code == 0 && results.manyResults_len != tr.manyDests_len) {
code = VOLSERFAILEDOP;
}
if (code) {
PrintError("Release failed: ", code);
} else {
@ -5325,7 +5339,7 @@ UV_ListOneVolume(afs_uint32 aserver, afs_int32 apart, afs_uint32 volid,
volumeInfo.volEntries_len = 0;
aconn = UV_Bind(aserver, AFSCONF_VOLUMEPORT);
code = AFSVolListOneVolume(aconn, apart, volid, &volumeInfo);
code = ListOneVolume(aconn, apart, volid, &volumeInfo);
if (code) {
fprintf(STDERR,
"Could not fetch the information about volume %lu from the server\n",
@ -5392,6 +5406,9 @@ UV_XListOneVolume(afs_uint32 a_serverID, afs_int32 a_partID, afs_uint32 a_volID,
*/
rxConnP = UV_Bind(a_serverID, AFSCONF_VOLUMEPORT);
code = AFSVolXListOneVolume(rxConnP, a_partID, a_volID, &volumeXInfo);
if (code == 0 && volumeXInfo.volXEntries_len != 1) {
code = VOLSERFAILEDOP;
}
if (code)
fprintf(STDERR,
"[UV_XListOneVolume] Couldn't fetch the volume information\n");
@ -6097,9 +6114,7 @@ UV_SyncVolume(afs_uint32 aserver, afs_int32 apart, char *avolname, int flags)
/* If a volume ID were given, search for it on each partition */
if ((volumeid = atol(avolname))) {
for (j = 0; j < pcnt; j++) {
code =
AFSVolListOneVolume(aconn, PartList.partId[j], volumeid,
&volumeInfo);
code = ListOneVolume(aconn, PartList.partId[j], volumeid, &volumeInfo);
if (code) {
if (code != ENODEV) {
fprintf(STDERR, "Could not query server\n");
@ -6137,9 +6152,8 @@ UV_SyncVolume(afs_uint32 aserver, afs_int32 apart, char *avolname, int flags)
for (k = 0; k < pcnt; k++) { /* For each partition */
volumeInfo.volEntries_val = (volintInfo *) 0;
volumeInfo.volEntries_len = 0;
code =
AFSVolListOneVolume(aconn, PartList.partId[k],
vldbentry.volumeId[j], &volumeInfo);
code = ListOneVolume(aconn, PartList.partId[k],
vldbentry.volumeId[j], &volumeInfo);
if (code) {
if (code != ENODEV) {
fprintf(STDERR, "Could not query server\n");
@ -6391,7 +6405,7 @@ VolumeExists(afs_uint32 server, afs_int32 partition, afs_uint32 volumeid)
if (conn) {
volumeInfo.volEntries_val = (volintInfo *) 0;
volumeInfo.volEntries_len = 0;
code = AFSVolListOneVolume(conn, partition, volumeid, &volumeInfo);
code = ListOneVolume(conn, partition, volumeid, &volumeInfo);
if (volumeInfo.volEntries_val)
free(volumeInfo.volEntries_val);
if (code == VOLSERILLEGAL_PARTITION)

3
src/volser/vsutils.c
View File

@ -377,6 +377,9 @@ VLDB_IsSameAddrs(afs_uint32 serv1, afs_uint32 serv2, afs_int32 *errorp)
}
code = 0;
if (addrs.bulkaddrs_len < nentries) {
nentries = addrs.bulkaddrs_len;
}
if (nentries > GETADDRUCACHESIZE)
nentries = GETADDRUCACHESIZE; /* safety check; should not happen */
if (++cacheip_index >= GETADDRUCACHESIZE)