From 9fa6c695b421f09090fdc81d1a1cadbb7d360126 Mon Sep 17 00:00:00 2001 From: divverent Date: Mon, 2 Mar 2015 21:25:42 +0000 Subject: [PATCH] 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 --- cl_input.c | 9 ++++++--- client.h | 7 +++---- clvm_cmds.c | 4 ++-- protocol.c | 2 +- protocol.h | 2 +- server.h | 6 +++--- sv_user.c | 14 +++++++++----- 7 files changed, 25 insertions(+), 19 deletions(-) diff --git a/cl_input.c b/cl_input.c index e5d2726f..b70a19ec 100644 --- 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 7c6fff4d..e254d81f 100644 --- 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 1d3696ac..9d41dda6 100644 --- 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 c0c8ac9d..de55ed37 100644 --- 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 3c0c6ce6..a3a26777 100644 --- 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 59d0eca3..8c54f97f 100644 --- 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 0cce2eb9..5f8bf86a 100644 --- 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 -- 2.39.2