From: bones_was_here <bones_was_here@xonotic.au>
Date: Sun, 5 May 2024 19:13:34 +0000 (+1000)
Subject: net: fix UB in VM_CL_getstati() and when sending the `items` stat
X-Git-Url: https://git.rm.cloudns.org/?a=commitdiff_plain;h=9aff57968fdea0b6792a8826d101788f31fdf1a3;p=xonotic%2Fdarkplaces.git

net: fix UB in VM_CL_getstati() and when sending the `items` stat

Simplifies the shifting in VM_CL_getstati() to avoid signed integer
overflow when the high bits are read with the arguments 23 and 9 (as
done in "quake15" and "ad" mods).

Refactors VM_CL_getstati() slightly, returns 0 when the index is out of
range (instead of whatever happened to be in that memory).

Masks off the bits that will overflow when packing the `items` stat for
sending (otherwise corruption of the low bits could occur on x86).

Signed-off-by: bones_was_here <bones_was_here@xonotic.au>
---

diff --git a/clvm_cmds.c b/clvm_cmds.c
index fac584d4..99f907d4 100644
--- a/clvm_cmds.c
+++ b/clvm_cmds.c
@@ -2017,6 +2017,7 @@ static void VM_CL_getstatf (prvm_prog_t *prog)
 	i = (int)PRVM_G_FLOAT(OFS_PARM0);
 	if(i < 0 || i >= MAX_CL_STATS)
 	{
+		PRVM_G_FLOAT(OFS_RETURN) = 0;
 		VM_Warning(prog, "VM_CL_getstatf: index>=MAX_CL_STATS or index<0\n");
 		return;
 	}
@@ -2027,12 +2028,18 @@ static void VM_CL_getstatf (prvm_prog_t *prog)
 //#331 float(float stnum) getstati (EXT_CSQC)
 static void VM_CL_getstati (prvm_prog_t *prog)
 {
-	int i, index;
-	int firstbit, bitcount;
+	int index, firstbit, bitcount;
 
 	VM_SAFEPARMCOUNTRANGE(1, 3, VM_CL_getstati);
 
 	index = (int)PRVM_G_FLOAT(OFS_PARM0);
+	if(index < 0 || index >= MAX_CL_STATS)
+	{
+		PRVM_G_FLOAT(OFS_RETURN) = 0;
+		VM_Warning(prog, "VM_CL_getstati: index>=MAX_CL_STATS or index<0\n");
+		return;
+	}
+
 	if (prog->argc > 1)
 	{
 		firstbit = (int)PRVM_G_FLOAT(OFS_PARM1);
@@ -2047,15 +2054,10 @@ static void VM_CL_getstati (prvm_prog_t *prog)
 		bitcount = 32;
 	}
 
-	if(index < 0 || index >= MAX_CL_STATS)
-	{
-		VM_Warning(prog, "VM_CL_getstati: index>=MAX_CL_STATS or index<0\n");
-		return;
-	}
-	i = cl.stats[index];
-	if (bitcount != 32)	//32 causes the mask to overflow, so there's nothing to subtract from.
-		i = (((unsigned int)i)&(((1<<bitcount)-1)<<firstbit))>>firstbit;
-	PRVM_G_FLOAT(OFS_RETURN) = i;
+	if (bitcount < 32)	//32 causes the mask to overflow, so there's nothing to subtract from.
+		PRVM_G_FLOAT(OFS_RETURN) = cl.stats[index]>>firstbit & ((1<<bitcount)-1);
+	else
+		PRVM_G_FLOAT(OFS_RETURN) = cl.stats[index];
 }
 
 //#332 string(float firststnum) getstats (EXT_CSQC)
diff --git a/sv_send.c b/sv_send.c
index a8a44347..ae2d9b9d 100644
--- a/sv_send.c
+++ b/sv_send.c
@@ -1145,7 +1145,9 @@ void SV_WriteClientdataToMessage (client_t *client, prvm_edict_t *ent, sizebuf_t
 	// the runes are in serverflags, pack them into the items value, also pack
 	// in the items2 value for mission pack huds
 	// (used only in the mission packs, which do not use serverflags)
-	items = (int)PRVM_serveredictfloat(ent, items) | ((int)PRVM_serveredictfloat(ent, items2) << 23) | ((int)PRVM_serverglobalfloat(serverflags) << 28);
+	items = (int)PRVM_serveredictfloat(ent, items)
+		| (((int)PRVM_serveredictfloat(ent, items2) & ((1<<9)-1)) << 23)
+		| (((int)PRVM_serverglobalfloat(serverflags) & ((1<<4)-1)) << 28);
 
 	VectorCopy(PRVM_serveredictvector(ent, punchvector), punchvector);