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: xdr: Avoid prealloc'd string OUT args

Browse Source 
CVE-2024-10397

Currently, several callers call RPCs with string OUT arguments, and
provide preallocated memory for those arguments. This can easily allow a
response from the server to overrun the allocated buffer, stomping over
stack or heap memory.

We could simply make our preallocated buffers larger than the maximum
size that the RPC allows, but relying on that is error prone, and
there's no way for XDR to check if a string buffer is large enough.

Instead, to make sure we don't overrun a given preallocated buffer,
avoid giving a preallocated buffer to such RPCs, and let XDR allocate
the memory for us.

Specifically, this commit changes several callers to
RXAFS_GetVolumeStatus(), and one caller of BOZO_GetInstanceParm(), to
avoid passing in a preallocated string buffer.

All other callers of RPCs with string OUT args already let XDR allocate
the buffers for them.

FIXES 135043

Change-Id: If42e2cc983903cff9766e1bab487142d4d493a17
Reviewed-on: https://gerrit.openafs.org/15918
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
This commit is contained in:
Andrew Deason 2024-06-13 15:28:38 -05:00 committed by Benjamin Kaduk
parent ac602a0a56
commit 00a1b266af
6 changed files with 77 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
src/WINNT/afsd/cm_ioctl.c
View File

@ -764,9 +764,6 @@ cm_IoctlGetVolumeStatus(struct cm_ioctl *ioctlp, struct cm_user *userp, cm_scach
cm_conn_t *connp;
AFSFetchVolumeStatus volStat;
char *cp;
char *Name;
char *OfflineMsg;
char *MOTD;
struct rx_connection * rxconnp;
#ifdef AFS_FREELANCE_CLIENT
@ -785,19 +782,23 @@ cm_IoctlGetVolumeStatus(struct cm_ioctl *ioctlp, struct cm_user *userp, cm_scach
{
cm_fid_t vfid;
cm_scache_t *vscp;
char *Name = NULL;
char *OfflineMsg = NULL;
char *MOTD = NULL;
cm_SetFid(&vfid, scp->fid.cell, scp->fid.volume, 1, 1);
code = cm_GetSCache(&vfid, NULL, &vscp, userp, reqp);
if (code)
return code;
Name = volName;
OfflineMsg = offLineMsg;
MOTD = motd;
do {
code = cm_ConnFromFID(&vfid, userp, reqp, &connp);
if (code) continue;
xdr_free((xdrproc_t) xdr_string, &Name);
xdr_free((xdrproc_t) xdr_string, &OfflineMsg);
xdr_free((xdrproc_t) xdr_string, &MOTD);
rxconnp = cm_GetRxConn(connp);
code = RXAFS_GetVolumeStatus(rxconnp, vfid.volume,
&volStat, &Name, &OfflineMsg, &MOTD);
@ -807,6 +808,19 @@ cm_IoctlGetVolumeStatus(struct cm_ioctl *ioctlp, struct cm_user *userp, cm_scach
code = cm_MapRPCError(code, reqp);
cm_ReleaseSCache(vscp);
strncpy(volName, Name, sizeof(volName));
volName[sizeof(volName) - 1] = '\0';
strncpy(offLineMsg, OfflineMsg, sizeof(offLineMsg));
offLineMsg[sizeof(offLineMsg) - 1] = '\0';
strncpy(motd, MOTD, sizeof(motd));
motd[sizeof(motd) - 1] = '\0';
xdr_free((xdrproc_t) xdr_string, &Name);
xdr_free((xdrproc_t) xdr_string, &OfflineMsg);
xdr_free((xdrproc_t) xdr_string, &MOTD);
}
if (code)

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

@ -2743,12 +2743,6 @@ cm_IsSpaceAvailable(cm_fid_t * fidp, osi_hyper_t *sizep, cm_user_t *userp, cm_re
AFSFetchVolumeStatus volStat;
cm_volume_t *volp = NULL;
afs_uint32 volType;
char *Name;
char *OfflineMsg;
char *MOTD;
char volName[32]="(unknown)";
char offLineMsg[256]="server temporarily inaccessible";
char motd[256]="server temporarily inaccessible";
osi_hyper_t freespace;
cm_fid_t vfid;
cm_scache_t *vscp;
@ -2778,11 +2772,11 @@ cm_IsSpaceAvailable(cm_fid_t * fidp, osi_hyper_t *sizep, cm_user_t *userp, cm_re
CM_SCACHESYNC_NEEDCALLBACK | CM_SCACHESYNC_GETSTATUS);
lock_ReleaseWrite(&vscp->rw);
if (code == 0) {
Name = volName;
OfflineMsg = offLineMsg;
MOTD = motd;
do {
char *Name = NULL;
char *OfflineMsg = NULL;
char *MOTD = NULL;
code = cm_ConnFromFID(&vfid, userp, reqp, &connp);
if (code) continue;
@ -2791,6 +2785,10 @@ cm_IsSpaceAvailable(cm_fid_t * fidp, osi_hyper_t *sizep, cm_user_t *userp, cm_re
&volStat, &Name, &OfflineMsg, &MOTD);
rx_PutConnection(rxconnp);
xdr_free((xdrproc_t) xdr_string, &Name);
xdr_free((xdrproc_t) xdr_string, &OfflineMsg);
xdr_free((xdrproc_t) xdr_string, &MOTD);
} while (cm_Analyze(connp, userp, reqp, &vfid, NULL, 0, NULL, NULL, NULL, NULL, code));
code = cm_MapRPCError(code, reqp);
}

19
src/WINNT/afsd/cm_volume.c
View File

@ -1310,24 +1310,14 @@ cm_CheckOfflineVolumeState(cm_volume_t *volp, cm_vol_state_t *statep, afs_uint32
cm_conn_t *connp;
long code;
AFSFetchVolumeStatus volStat;
char *Name;
char *OfflineMsg;
char *MOTD;
cm_req_t req;
struct rx_connection * rxconnp;
char volName[32];
afs_uint32 volType;
char offLineMsg[256];
char motd[256];
long alldown, alldeleted;
cm_serverRef_t *serversp;
cm_fid_t vfid;
cm_scache_t *vscp = NULL;
Name = volName;
OfflineMsg = offLineMsg;
MOTD = motd;
volType = cm_VolumeType(volp, volID);
if (statep->ID != 0 && (!volID || volID == statep->ID)) {
@ -1373,6 +1363,10 @@ cm_CheckOfflineVolumeState(cm_volume_t *volp, cm_vol_state_t *statep, afs_uint32
code = cm_GetSCache(&vfid, NULL, &vscp, cm_rootUserp, &req);
if (code = 0) {
do {
char *Name = NULL;
char *OfflineMsg = NULL;
char *MOTD = NULL;
code = cm_ConnFromVolume(volp, statep->ID, cm_rootUserp, &req, &connp);
if (code)
continue;
@ -1381,6 +1375,11 @@ cm_CheckOfflineVolumeState(cm_volume_t *volp, cm_vol_state_t *statep, afs_uint32
code = RXAFS_GetVolumeStatus(rxconnp, statep->ID,
&volStat, &Name, &OfflineMsg, &MOTD);
rx_PutConnection(rxconnp);
xdr_free((xdrproc_t) xdr_string, &Name);
xdr_free((xdrproc_t) xdr_string, &OfflineMsg);
xdr_free((xdrproc_t) xdr_string, &MOTD);
} while (cm_Analyze(connp, cm_rootUserp, &req, &vfid, NULL, 0, NULL, NULL, NULL, NULL, code));
code = cm_MapRPCError(code, &req);

34
src/WINNT/afsrdr/user/RDRFunction.c
View File

@ -5820,14 +5820,8 @@ RDR_GetVolumeInfo( IN cm_user_t *userp,
DWORD status;
FILETIME ft = {0x832cf000, 0x01abfcc4}; /* October 1, 1982 00:00:00 +0600 */
char volName[32]="(unknown)";
char offLineMsg[256]="server temporarily inaccessible";
char motd[256]="server temporarily inaccessible";
cm_conn_t *connp;
AFSFetchVolumeStatus volStat;
char *Name;
char *OfflineMsg;
char *MOTD;
struct rx_connection * rxconnp;
int scp_locked = 0;
@ -5928,13 +5922,14 @@ RDR_GetVolumeInfo( IN cm_user_t *userp,
if (code == -1)
{
Name = volName;
OfflineMsg = offLineMsg;
MOTD = motd;
lock_ReleaseWrite(&scp->rw);
scp_locked = 0;
do {
char *Name = NULL;
char *OfflineMsg = NULL;
char *MOTD = NULL;
code = cm_ConnFromFID(&scp->fid, userp, &req, &connp);
if (code) continue;
@ -5943,6 +5938,10 @@ RDR_GetVolumeInfo( IN cm_user_t *userp,
&volStat, &Name, &OfflineMsg, &MOTD);
rx_PutConnection(rxconnp);
xdr_free((xdrproc_t) xdr_string, &Name);
xdr_free((xdrproc_t) xdr_string, &OfflineMsg);
xdr_free((xdrproc_t) xdr_string, &MOTD);
} while (cm_Analyze(connp, userp, &req, &scp->fid, NULL, 0, NULL, NULL, NULL, NULL, code));
code = cm_MapRPCError(code, &req);
@ -6056,14 +6055,8 @@ RDR_GetVolumeSizeInfo( IN cm_user_t *userp,
cm_req_t req;
DWORD status;
char volName[32]="(unknown)";
char offLineMsg[256]="server temporarily inaccessible";
char motd[256]="server temporarily inaccessible";
cm_conn_t *connp;
AFSFetchVolumeStatus volStat;
char *Name;
char *OfflineMsg;
char *MOTD;
struct rx_connection * rxconnp;
int scp_locked = 0;
@ -6141,13 +6134,14 @@ RDR_GetVolumeSizeInfo( IN cm_user_t *userp,
if (code == -1)
{
Name = volName;
OfflineMsg = offLineMsg;
MOTD = motd;
lock_ReleaseWrite(&scp->rw);
scp_locked = 0;
do {
char *Name = NULL;
char *OfflineMsg = NULL;
char *MOTD = NULL;
code = cm_ConnFromFID(&scp->fid, userp, &req, &connp);
if (code) continue;
@ -6156,6 +6150,10 @@ RDR_GetVolumeSizeInfo( IN cm_user_t *userp,
&volStat, &Name, &OfflineMsg, &MOTD);
rx_PutConnection(rxconnp);
xdr_free((xdrproc_t) xdr_string, &Name);
xdr_free((xdrproc_t) xdr_string, &OfflineMsg);
xdr_free((xdrproc_t) xdr_string, &MOTD);
} while (cm_Analyze(connp, userp, &req, &scp->fid, NULL, 0, NULL, NULL, NULL, NULL, code));
code = cm_MapRPCError(code, &req);

20
src/afs/afs_pioctl.c
View File

@ -1997,32 +1997,31 @@ DECL_PIOCTL(PSetTokens)
*/
DECL_PIOCTL(PGetVolumeStatus)
{
char volName[32];
char *offLineMsg = afs_osi_Alloc(256);
char *motd = afs_osi_Alloc(256);
char *volName = NULL;
char *offLineMsg = NULL;
char *motd = NULL;
struct afs_conn *tc;
afs_int32 code = 0;
struct AFSFetchVolumeStatus volstat;
char *Name;
struct rx_connection *rxconn;
XSTATS_DECLS;
osi_Assert(offLineMsg != NULL);
osi_Assert(motd != NULL);
AFS_STATCNT(PGetVolumeStatus);
if (!avc) {
code = EINVAL;
goto out;
}
Name = volName;
do {
tc = afs_Conn(&avc->f.fid, areq, SHARED_LOCK, &rxconn);
if (tc) {
XSTATS_START_TIME(AFS_STATS_FS_RPCIDX_GETVOLUMESTATUS);
RX_AFS_GUNLOCK();
xdr_free((xdrproc_t) xdr_string, &volName);
xdr_free((xdrproc_t) xdr_string, &offLineMsg);
xdr_free((xdrproc_t) xdr_string, &motd);
code =
RXAFS_GetVolumeStatus(rxconn, avc->f.fid.Fid.Volume, &volstat,
&Name, &offLineMsg, &motd);
&volName, &offLineMsg, &motd);
RX_AFS_GLOCK();
XSTATS_END_TIME;
} else
@ -2043,8 +2042,9 @@ DECL_PIOCTL(PGetVolumeStatus)
if (afs_pd_putString(aout, motd) != 0)
return E2BIG;
out:
afs_osi_Free(offLineMsg, 256);
afs_osi_Free(motd, 256);
xdr_free((xdrproc_t) xdr_string, &volName);
xdr_free((xdrproc_t) xdr_string, &offLineMsg);
xdr_free((xdrproc_t) xdr_string, &motd);
return code;
}

18
src/libadmin/bos/afs_bosAdmin.c
View File

@ -1289,6 +1289,8 @@ bos_ProcessNotifierGet(const void *serverHandle, const char *processName,
int rc = 0;
afs_status_t tst = 0;
bos_server_p b_handle = (bos_server_p) serverHandle;
char *tnotif = NULL;
size_t len;
if (!isValidServerHandle(b_handle, &tst)) {
goto fail_bos_ProcessNotifierGet;
@ -1305,14 +1307,22 @@ bos_ProcessNotifierGet(const void *serverHandle, const char *processName,
}
tst = BOZO_GetInstanceParm(b_handle->server, (char *)processName,
999, &notifier);
if (tst == 0) {
rc = 1;
999, &tnotif);
if (tst != 0) {
goto fail_bos_ProcessNotifierGet;
}
len = strlcpy(notifier, tnotif, BOS_MAX_NAME_LEN);
if (len >= BOS_MAX_NAME_LEN) {
tst = ADMRPCTOOBIG;
goto fail_bos_ProcessNotifierGet;
}
rc = 1;
fail_bos_ProcessNotifierGet:
xdr_free((xdrproc_t)xdr_string, &tnotif);
if (st != NULL) {
*st = tst;
}