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 fac584d475a455ead9697c3070fb65e53f4b48ac..99f907d492400e1e5e891fd8ae8a6567dc6c6a2c 100644 (file)
--- 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 a8a44347724791acfc0f179655e2c010ae2dd65a..ae2d9b9df38271742c26bf0bd829ef22d5a178d7 100644 (file)
--- 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);