Make the move sequence an unsigned int.

This fixes an out-of-bounds write to movement_count because C's modulo
operation is considered harmful.

Many thanks to afl-fuzz!

git-svn-id: svn://svn.icculus.org/twilight/trunk/darkplaces@12170 d7cf8633-e32d-0410-b094-e92efae38249

diff --git a/cl_input.c b/cl_input.c
index e5d2726fd472181a2825ba56ffb0c011c2093121..b70a19eccbcbe834b9950077f8775c52c9d8bc3a 100644 (file)
--- a/cl_input.c
+++ b/cl_input.c
@@ -1601,7 +1601,7 @@ void CL_ClientMovement_Replay(void)
                if (cl.movecmd[i].sequence > cls.servermovesequence)
                        totalmovemsec += cl.movecmd[i].msec;
        cl.movement_predicted = totalmovemsec >= cl_movement_minping.value && cls.servermovesequence && (cl_movement.integer && !cls.demoplayback && cls.signon == SIGNONS && cl.stats[STAT_HEALTH] > 0 && !cl.intermission);
-       //Con_Printf("%i = %.0f >= %.0f && %i && (%i && %i && %i == %i && %i > 0 && %i\n", cl.movement_predicted, totalmovemsec, cl_movement_minping.value, cls.servermovesequence, cl_movement.integer, !cls.demoplayback, cls.signon, SIGNONS, cl.stats[STAT_HEALTH], !cl.intermission);
+       //Con_Printf("%i = %.0f >= %.0f && %u && (%i && %i && %i == %i && %i > 0 && %i\n", cl.movement_predicted, totalmovemsec, cl_movement_minping.value, cls.servermovesequence, cl_movement.integer, !cls.demoplayback, cls.signon, SIGNONS, cl.stats[STAT_HEALTH], !cl.intermission);
        if (cl.movement_predicted)
        {
                //Con_Printf("%ims\n", cl.movecmd[0].msec);
@@ -2060,8 +2060,11 @@ void CL_SendMove(void)
                // framerate, this is 10 bytes, if client framerate is lower this
                // will be more...
                int i, j;
-               int oldsequence = cl.cmd.sequence - bound(1, cl_netrepeatinput.integer + 1, 3);
-               if (oldsequence < 1)
+               unsigned int oldsequence = cl.cmd.sequence;
+               unsigned int delta = bound(1, cl_netrepeatinput.integer + 1, 3);
+               if (oldsequence > delta)
+                       oldsequence = oldsequence - delta;
+               else
                        oldsequence = 1;
                for (i = 0;i < LATESTFRAMENUMS;i++)
                {

diff --git a/client.h b/client.h
index 7c6fff4da024fce39d867532b8f21aeb259b7abb..e254d81fc183e440d8eddbbecbf37252b6577e57 100644 (file)
--- a/client.h
+++ b/client.h
@@ -638,7 +638,7 @@ typedef struct usercmd_s
        int msec; // for predicted moves
        int buttons;
        int impulse;
-       int sequence;
+       unsigned int sequence;
        qboolean applied; // if false we're still accumulating a move
        qboolean predicted; // if true the sequence should be sent as 0
 
@@ -856,8 +856,7 @@ typedef struct client_static_s
        cl_downloadack_t dp_downloadack[CL_MAX_DOWNLOADACKS];
 
        // input sequence numbers are not reset on level change, only connect
-       int movesequence;
-       int servermovesequence;
+       unsigned int servermovesequence;
 
        // quakeworld stuff below
 
@@ -1275,7 +1274,7 @@ typedef struct client_state_s
 #define LATESTFRAMENUMS 32
        int latestframenumsposition;
        int latestframenums[LATESTFRAMENUMS];
-       int latestsendnums[LATESTFRAMENUMS];
+       unsigned int latestsendnums[LATESTFRAMENUMS];
        entityframe_database_t *entitydatabase;
        entityframe4_database_t *entitydatabase4;
        entityframeqw_database_t *entitydatabaseqw;

diff --git a/clvm_cmds.c b/clvm_cmds.c
index 1d3696acab493ab69a398938aa55bf7928d355ed..9d41dda6d454fc4579c01839ef7f5b03e3ce6f9e 100644 (file)
--- a/clvm_cmds.c
+++ b/clvm_cmds.c
@@ -1446,9 +1446,9 @@ static void VM_CL_getmousepos(prvm_prog_t *prog)
 //#345 float(float framenum) getinputstate (EXT_CSQC)
 static void VM_CL_getinputstate (prvm_prog_t *prog)
 {
-       int i, frame;
+       unsigned int i, frame;
        VM_SAFEPARMCOUNT(1, VM_CL_getinputstate);
-       frame = (int)PRVM_G_FLOAT(OFS_PARM0);
+       frame = (unsigned int)PRVM_G_FLOAT(OFS_PARM0);
        PRVM_G_FLOAT(OFS_RETURN) = false;
        for (i = 0;i < CL_MAX_USERCMDS;i++)
        {

diff --git a/protocol.c b/protocol.c
index c0c8ac9d7e78ad07a44ab4e6862c6243cf7602f2..de55ed3750fbf2510da1d83bae2bf4a4f05ebbdc 100644 (file)
--- a/protocol.c
+++ b/protocol.c
@@ -2832,7 +2832,7 @@ void EntityFrame5_AckFrame(entityframe5_database_t *d, int framenum)
                        d->packetlog[i].packetnumber = 0;
 }
 
-qboolean EntityFrame5_WriteFrame(sizebuf_t *msg, int maxsize, entityframe5_database_t *d, int numstates, const entity_state_t **states, int viewentnum, int movesequence, qboolean need_empty)
+qboolean EntityFrame5_WriteFrame(sizebuf_t *msg, int maxsize, entityframe5_database_t *d, int numstates, const entity_state_t **states, int viewentnum, unsigned int movesequence, qboolean need_empty)
 {
        prvm_prog_t *prog = SVVM_prog;
        const entity_state_t *n;

diff --git a/protocol.h b/protocol.h
index 3c0c6ce68dbdbc7af78a9a1433ba8dd6ef18f95a..a3a267774ecfd1ce120068f1e00bc53166af1637 100644 (file)
--- a/protocol.h
+++ b/protocol.h
@@ -837,7 +837,7 @@ int EntityState5_DeltaBitsForState(entity_state_t *o, entity_state_t *n);
 void EntityFrame5_CL_ReadFrame(void);
 void EntityFrame5_LostFrame(entityframe5_database_t *d, int framenum);
 void EntityFrame5_AckFrame(entityframe5_database_t *d, int framenum);
-qboolean EntityFrame5_WriteFrame(sizebuf_t *msg, int maxsize, entityframe5_database_t *d, int numstates, const entity_state_t **states, int viewentnum, int movesequence, qboolean need_empty);
+qboolean EntityFrame5_WriteFrame(sizebuf_t *msg, int maxsize, entityframe5_database_t *d, int numstates, const entity_state_t **states, int viewentnum, unsigned int movesequence, qboolean need_empty);
 
 extern cvar_t developer_networkentities;
 

diff --git a/server.h b/server.h
index 59d0eca3bf9ba3ebbe5590f003ee336e8722a8e4..8c54f97f6f9da2ca9f9f2247f28a89a6f7e44627 100644 (file)
--- a/server.h
+++ b/server.h
@@ -214,9 +214,9 @@ typedef struct client_s
        /// communications handle
        netconn_t *netconnection;
 
-       int movesequence;
+       unsigned int movesequence;
        signed char movement_count[NETGRAPH_PACKETS];
-       int movement_highestsequence_seen; // not the same as movesequence if prediction is off
+       unsigned int movement_highestsequence_seen; // not the same as movesequence if prediction is off
        /// movement
        usercmd_t cmd;
        /// intended motion calced from cmd
@@ -312,7 +312,7 @@ typedef struct client_s
 
        // last sent move sequence
        // if the move sequence changed, an empty entity frame is sent
-       int lastmovesequence;
+       unsigned int lastmovesequence;
 } client_t;
 
 

diff --git a/sv_user.c b/sv_user.c
index 0cce2eb9d84129266dd4b48e5ffbafd4f787ee2f..5f8bf86a02d65be8e0d1fbdda6d3b97849859265 100644 (file)
--- a/sv_user.c
+++ b/sv_user.c
@@ -465,7 +465,7 @@ static void SV_ReadClientMove (void)
        move->receivetime = (float)sv.time;
 
 #if DEBUGMOVES
-       Con_Printf("%s move%i #%i %ims (%ims) %i %i '%i %i %i' '%i %i %i'\n", move->time > move->receivetime ? "^3read future" : "^4read normal", sv_numreadmoves + 1, move->sequence, (int)floor((move->time - host_client->cmd.time) * 1000.0 + 0.5), (int)floor(move->time * 1000.0 + 0.5), move->impulse, move->buttons, (int)move->viewangles[0], (int)move->viewangles[1], (int)move->viewangles[2], (int)move->forwardmove, (int)move->sidemove, (int)move->upmove);
+       Con_Printf("%s move%i #%u %ims (%ims) %i %i '%i %i %i' '%i %i %i'\n", move->time > move->receivetime ? "^3read future" : "^4read normal", sv_numreadmoves + 1, move->sequence, (int)floor((move->time - host_client->cmd.time) * 1000.0 + 0.5), (int)floor(move->time * 1000.0 + 0.5), move->impulse, move->buttons, (int)move->viewangles[0], (int)move->viewangles[1], (int)move->viewangles[2], (int)move->forwardmove, (int)move->sidemove, (int)move->upmove);
 #endif
        // limit reported time to current time
        // (incase the client is trying to cheat)
@@ -551,9 +551,13 @@ static void SV_ReadClientMove (void)
                        if(host_client->movement_highestsequence_seen)
                        {
                                // mark moves in between as lost
-                               if(move->sequence - host_client->movement_highestsequence_seen - 1 < NETGRAPH_PACKETS)
-                                       for(i = host_client->movement_highestsequence_seen + 1; i < move->sequence; ++i)
-                                               host_client->movement_count[i % NETGRAPH_PACKETS] = -1;
+                               unsigned int delta = move->sequence - host_client->movement_highestsequence_seen - 1;
+                               if(delta < NETGRAPH_PACKETS)
+                               {
+                                       unsigned int u;
+                                       for(u = 0; u < delta; ++u)
+                                               host_client->movement_count[(host_client->movement_highestsequence_seen + 1 + u) % NETGRAPH_PACKETS] = -1;
+                               }
                                else
                                        memset(host_client->movement_count, -1, sizeof(host_client->movement_count));
                        }
@@ -608,7 +612,7 @@ static void SV_ExecuteClientMoves(void)
                        if (host_client->movesequence < move->sequence || moveindex == sv_numreadmoves - 1)
                        {
 #if DEBUGMOVES
-                               Con_Printf("%smove #%i %ims (%ims) %i %i '%i %i %i' '%i %i %i'\n", (move->time - host_client->cmd.time) > sv.frametime * 1.01 ? "^1" : "^2", move->sequence, (int)floor((move->time - host_client->cmd.time) * 1000.0 + 0.5), (int)floor(move->time * 1000.0 + 0.5), move->impulse, move->buttons, (int)move->viewangles[0], (int)move->viewangles[1], (int)move->viewangles[2], (int)move->forwardmove, (int)move->sidemove, (int)move->upmove);
+                               Con_Printf("%smove #%u %ims (%ims) %i %i '%i %i %i' '%i %i %i'\n", (move->time - host_client->cmd.time) > sv.frametime * 1.01 ? "^1" : "^2", move->sequence, (int)floor((move->time - host_client->cmd.time) * 1000.0 + 0.5), (int)floor(move->time * 1000.0 + 0.5), move->impulse, move->buttons, (int)move->viewangles[0], (int)move->viewangles[1], (int)move->viewangles[2], (int)move->forwardmove, (int)move->sidemove, (int)move->upmove);
 #endif
                                // this is a new move
                                move->time = bound(sv.time - 1, move->time, sv.time); // prevent slowhack/speedhack combos