From b5856e8be02370953b62ac99e6d4e36fd4d3a1ba Mon Sep 17 00:00:00 2001
From: Adam Olsen <rhamph@gmail.com>
Date: Thu, 25 Oct 2001 23:26:33 +0000
Subject: [PATCH] - audit of the net_svc.c users' sanity checking
---
qw/source/cl_ents.c | 64 +++++++++++++-----
qw/source/cl_parse.c | 154 +++++++++++++++++++++++++++++++------------
qw/source/cl_tent.c | 14 +++-
qw/source/cl_view.c | 5 +-
qw/source/net_svc.c | 6 +-
5 files changed, 179 insertions(+), 64 deletions(-)
diff --git a/qw/source/cl_ents.c b/qw/source/cl_ents.c
index b757f1ce6..74c4e218f 100644
--- a/qw/source/cl_ents.c
+++ b/qw/source/cl_ents.c
@@ -338,8 +338,8 @@ CL_ParsePacketEntities (void)
}
if (index >= MAX_PACKET_ENTITIES) {
- Host_NetError ("CL_ParsePacketEntities: index == "
- "MAX_PACKET_ENTITIES");
+ Host_NetError ("CL_ParsePacketEntities: index %i >= "
+ "MAX_PACKET_ENTITIES", index);
return;
}
@@ -350,6 +350,11 @@ CL_ParsePacketEntities (void)
&newp->entities[index],
block.vars[index].state.flags);
newp->entities[index].number = block.vars[index].state.number;
+ if (newp->entities[index].modelindex >= MAX_MODELS) {
+ Host_NetError ("CL_ParsePacketEntities: modelindex %i >= "
+ "MAX_MODELS", newp->entities[index].modelindex);
+ return;
+ }
continue;
}
@@ -386,7 +391,7 @@ CL_ParseDeltaPacketEntities ()
oldpacket = cl.frames[newpacket].delta_sequence;
if ((from & UPDATE_MASK) != (oldpacket & UPDATE_MASK))
- Con_DPrintf ("WARNING: from mismatch\n");
+ Con_DPrintf ("CL_ParseDeltaPacketEntities: WARNING: from mismatch\n");
full = false;
if (oldpacket != -1) {
@@ -410,16 +415,11 @@ CL_ParseDeltaPacketEntities ()
for (i = 0;; i++) {
word = block.vars[i].word;
- if (net_message->badread) { // something didn't parse right...
- Host_NetError ("msg_badread in packetentities");
- return;
- }
-
if (!word) { // copy rest of ents from old packet
while (oldindex < oldp->num_entities) {
// Con_Printf ("copy %i\n", oldp->entities[oldindex].number);
if (newindex >= MAX_PACKET_ENTITIES) {
- Host_NetError ("CL_ParseDeltaPacketEntities: newindex == "
+ Host_NetError ("CL_ParseDeltaPacketEntities: newindex >= "
"MAX_PACKET_ENTITIES (1st)");
return;
}
@@ -436,13 +436,14 @@ CL_ParseDeltaPacketEntities ()
while (newnum > oldnum) {
// if (newnum > oldnum) {
if (full) {
- Con_Printf ("WARNING: oldcopy on full update");
+ Con_Printf ("CL_ParseDeltaPacketEntities: WARNING: "
+ "oldcopy on full update");
return;
}
// Con_Printf ("copy %i\n", oldnum);
// copy one of the old entities over to the new packet unchanged
if (newindex >= MAX_PACKET_ENTITIES) {
- Host_NetError ("CL_ParseDeltaPacketEntities: newindex == "
+ Host_NetError ("CL_ParseDeltaPacketEntities: newindex >= "
"MAX_PACKET_ENTITIES (2nd)");
return;
}
@@ -458,14 +459,15 @@ CL_ParseDeltaPacketEntities ()
if (word & U_REMOVE) {
if (full) {
cl.validsequence = 0;
- Con_Printf ("WARNING: U_REMOVE on full update\n");
+ Con_Printf ("CL_ParseDeltaPacketEntities: "
+ "WARNING: U_REMOVE on full update\n");
return;
}
continue;
}
if (newindex >= MAX_PACKET_ENTITIES) {
- Host_NetError ("CL_ParseDeltaPacketEntities: newindex == "
+ Host_NetError ("CL_ParseDeltaPacketEntities: newindex >= "
"MAX_PACKET_ENTITIES (3rd)");
return;
}
@@ -476,6 +478,12 @@ CL_ParseDeltaPacketEntities ()
&newp->entities[newindex],
block.vars[i].state.flags);
newp->entities[newindex].number = block.vars[i].state.number;
+ if (newp->entities[newindex].modelindex >= MAX_MODELS) {
+ Host_NetError ("CL_ParsePacketEntities: modelindex %i >= "
+ "MAX_MODELS",
+ newp->entities[newindex].modelindex);
+ return;
+ }
newindex++;
continue;
}
@@ -499,6 +507,13 @@ CL_ParseDeltaPacketEntities ()
&newp->entities[newindex],
block.vars[i].state.flags);
newp->entities[newindex].number = block.vars[i].state.number;
+ if (newp->entities[newindex].modelindex >= MAX_MODELS) {
+ Host_NetError ("CL_ParsePacketEntities: modelindex %i >= "
+ "MAX_MODELS",
+ newp->entities[newindex].modelindex);
+ return;
+ }
+
newindex++;
oldindex++;
}
@@ -686,7 +701,10 @@ CL_ParseProjectiles (void)
projectile_t *pr;
net_svc_nails_t block;
- NET_SVC_Nails_Parse (&block, net_message);
+ if (NET_SVC_Nails_Parse (&block, net_message)) {
+ Host_NetError ("CL_ParseProjectiles: Bad Read\n");
+ return;
+ }
for (i = 0; i < block.numnails; i++) {
if (cl_num_projectiles == MAX_PROJECTILES)
@@ -742,11 +760,21 @@ CL_ParsePlayerinfo (void)
player_state_t *state;
net_svc_playerinfo_t block;
- NET_SVC_Playerinfo_Parse (&block, net_message);
+ if (NET_SVC_Playerinfo_Parse (&block, net_message)) {
+ Host_NetError ("CL_ParsePlayerinfo: Bad Read\n");
+ return;
+ }
- if (block.playernum > MAX_CLIENTS)
-// Sys_Error ("CL_ParsePlayerinfo: bad block.playernum");
- Host_NetError ("CL_ParsePlayerinfo: bad block.playernum");
+ if (block.playernum >= MAX_CLIENTS) {
+ Host_NetError ("CL_ParsePlayerinfo: playernum %i >= MAX_CLIENTS",
+ block.playernum);
+ return;
+ }
+ if (block.modelindex >= MAX_MODELS) {
+ Host_NetError ("CL_ParsePlayerinfo: modelindex %i >= MAX_MODELS",
+ block.modelindex);
+ return;
+ }
state = &cl.frames[parsecountmod].playerstate[block.playernum];
diff --git a/qw/source/cl_parse.c b/qw/source/cl_parse.c
index 96450b63e..6c3fcc26e 100644
--- a/qw/source/cl_parse.c
+++ b/qw/source/cl_parse.c
@@ -408,12 +408,14 @@ CL_ParseDownload (void)
int r;
net_svc_download_t download;
- NET_SVC_Download_Parse (&download, net_message);
-
- if (cls.demoplayback) {
- return; // not in demo playback
+ if (NET_SVC_Download_Parse (&download, net_message)) {
+ Host_NetError ("CL_ParseDownload: Bad Read\n");
+ return;
}
+ if (cls.demoplayback)
+ return; // not in demo playback
+
if (download.size == -1) {
Con_Printf ("File not found.\n");
if (cls.download) {
@@ -426,13 +428,11 @@ CL_ParseDownload (void)
}
if (download.size == -2) {
- const char *newname = download.name;
-
- if (strncmp (newname, cls.downloadname, strlen (cls.downloadname))
- || strstr (newname + strlen (cls.downloadname), "/")) {
- Con_Printf
- ("WARNING: server tried to give a strange new name: %s\n",
- newname);
+ // don't compare past the end of cls.downloadname due to gz support
+ if (strncmp (download.name, cls.downloadname, strlen (cls.downloadname))
+ || strstr (download.name + strlen (cls.downloadname), "/")) {
+ Con_Printf ("WARNING: server tried to give a strange new "
+ "name: %s\n", download.name);
CL_RequestNextDownload ();
return;
}
@@ -440,13 +440,15 @@ CL_ParseDownload (void)
Qclose (cls.download);
unlink (cls.downloadname);
}
- strncpy (cls.downloadname, newname, sizeof (cls.downloadname));
+ strncpy (cls.downloadname, download.name,
+ sizeof (cls.downloadname) - 1);
Con_Printf ("downloading to %s\n", cls.downloadname);
return;
}
if (download.size <= 0) {
Host_NetError ("Bad download block, size %d", download.size);
+ return;
}
// open the file if not opened yet
@@ -609,7 +611,10 @@ CL_ParsePrint (void)
char tmpstring[2048];
net_svc_print_t print;
- NET_SVC_Print_Parse (&print, net_message);
+ if (NET_SVC_Print_Parse (&print, net_message)) {
+ Host_NetError ("CL_ParsePrint: Bad Read\n");
+ return;
+ }
string = print.message;
if (print.level == PRINT_CHAT) {
@@ -645,7 +650,6 @@ void
CL_ParseServerData (void)
{
char fn[MAX_OSPATH];
- const char *str;
int protover;
qboolean cflag = false;
net_svc_serverdata_t serverdata;
@@ -654,7 +658,10 @@ CL_ParseServerData (void)
Con_DPrintf ("Serverdata packet received.\n");
- NET_SVC_ServerData_Parse (&serverdata, net_message);
+ if (NET_SVC_ServerData_Parse (&serverdata, net_message)) {
+ Host_NetError ("CL_ParseServerData: Bad Read\n");
+ return;
+ }
// wipe the client_state_t struct
CL_ClearState ();
@@ -672,16 +679,14 @@ CL_ParseServerData (void)
cl.servercount = serverdata.servercount;
// game directory
- str = serverdata.gamedir;
-
- if (!strequal (gamedirfile, str)) {
+ if (!strequal (gamedirfile, serverdata.gamedir)) {
// save current config
Host_WriteConfiguration ();
cflag = true;
Draw_ClearCache ();
}
- COM_Gamedir (str);
+ COM_Gamedir (serverdata.gamedir);
// ZOID--run the autoexec.cfg in the gamedir
// if it exists
@@ -701,14 +706,18 @@ CL_ParseServerData (void)
// parse player slot
cl.playernum = serverdata.playernum;
cl.spectator = serverdata.spectator;
+ if (cl.playernum >= MAX_CLIENTS) {
+ Host_NetError ("CL_ParseServerData: playernum %d >= MAX_CLIENTS",
+ cl.playernum);
+ return;
+ }
// FIXME: evil hack so NQ and QW can share sound code
cl.viewentity = cl.playernum + 1;
snd_viewentity = cl.playernum + 1;
// get the full level name
- str = serverdata.levelname;
- strncpy (cl.levelname, str, sizeof (cl.levelname) - 1);
+ strncpy (cl.levelname, serverdata.levelname, sizeof (cl.levelname) - 1);
// get the movevars
memcpy (&movevars, &serverdata.movevars, sizeof (movevars));
@@ -716,7 +725,7 @@ CL_ParseServerData (void)
// seperate the printfs so the server message can have a color
Con_Printf ("\n\n\35\36\36\36\36\36\36\36\36\36\36\36\36\36\36\36\36\36\36"
"\36\36\36\36\36\36\36\36\36\36\36\36\36\36\36\36\36\37\n\n");
- Con_Printf ("%c%s\n", 2, str);
+ Con_Printf ("%c%s\n", 2, serverdata.levelname);
// ask for the sound list next
memset (cl.sound_name, 0, sizeof (cl.sound_name));
@@ -754,15 +763,20 @@ CL_ParseSoundlist (void)
net_svc_soundlist_t soundlist;
// precache sounds
- NET_SVC_Soundlist_Parse (&soundlist, net_message);
+ if (NET_SVC_Soundlist_Parse (&soundlist, net_message)) {
+ Host_NetError ("CL_ParseSoundlist: Bad Read\n");
+ return;
+ }
for (i = 0, numsounds = soundlist.startsound;; i++) {
str = soundlist.sounds[i];
if (!str[0])
break;
numsounds++;
- if (numsounds == MAX_SOUNDS)
- Host_NetError ("Server sent too many sound_precache");
+ if (numsounds >= MAX_SOUNDS) {
+ Host_NetError ("CL_ParseSoundlist: too many sounds");
+ return;
+ }
strcpy (cl.sound_name[numsounds], str);
}
@@ -787,15 +801,20 @@ CL_ParseModellist (void)
net_svc_modellist_t modellist;
// precache models and note certain default indexes
- NET_SVC_Modellist_Parse (&modellist, net_message);
+ if (NET_SVC_Modellist_Parse (&modellist, net_message)) {
+ Host_NetError ("CL_ParseModellist: Bad Read\n");
+ return;
+ }
for (i = 0, nummodels = modellist.startmodel;; i++) {
str = modellist.models[i];
if (!str[0])
break;
nummodels++;
- if (nummodels == MAX_MODELS)
- Host_NetError ("Server sent too many model_precache");
+ if (nummodels >= MAX_MODELS) {
+ Host_NetError ("CL_ParseModellist: too many models");
+ return;
+ }
strncpy (cl.model_name[nummodels], str, MAX_QPATH - 1);
cl.model_name[nummodels][MAX_QPATH - 1] = '\0';
@@ -835,7 +854,21 @@ CL_ParseSpawnBaseline ()
entity_state_t *es;
net_svc_spawnbaseline_t block;
- NET_SVC_SpawnBaseline_Parse (&block, net_message);
+ if (NET_SVC_SpawnBaseline_Parse (&block, net_message)) {
+ Host_NetError ("CL_ParseSpawnBaseline: Bad Read\n");
+ return;
+ }
+
+ if (block.num >= MAX_EDICTS) {
+ Host_NetError ("CL_ParseSpawnBaseline: num %i >= MAX_EDICTS",
+ block.num);
+ return;
+ }
+ if (block.modelindex >= MAX_MODELS) {
+ Host_NetError ("CL_ParseSpawnBaseline: modelindex %i >= MAX_MODELS",
+ block.modelindex);
+ return;
+ }
es = &cl_baselines[block.num];
@@ -867,10 +900,21 @@ CL_ParseStatic (void)
entity_t *ent;
net_svc_spawnstatic_t block;
- NET_SVC_SpawnStatic_Parse (&block, net_message);
+ if (NET_SVC_SpawnStatic_Parse (&block, net_message)) {
+ Host_NetError ("CL_ParseStatic: Bad Read\n");
+ return;
+ }
- if (cl.num_statics >= MAX_STATIC_ENTITIES)
+ if (block.modelindex >= MAX_MODELS) {
+ Host_NetError ("CL_ParseStatic: modelindex %i >= MAX_MODELS",
+ block.modelindex);
+ return;
+ }
+
+ if (cl.num_statics >= MAX_STATIC_ENTITIES) {
Host_NetError ("Too many static entities");
+ return;
+ }
ent = &cl_static_entities[cl.num_statics++];
CL_Init_Entity (ent);
@@ -890,7 +934,10 @@ CL_ParseStaticSound (void)
{
net_svc_spawnstaticsound_t block;
- NET_SVC_SpawnStaticSound_Parse (&block, net_message);
+ if (NET_SVC_SpawnStaticSound_Parse (&block, net_message)) {
+ Host_NetError ("CL_ParseStaticSound: Bad Read\n");
+ return;
+ }
S_StaticSound (cl.sound_precache[block.sound_num], block.position,
block.volume, block.attenuation);
@@ -903,10 +950,20 @@ CL_ParseStartSoundPacket (void)
{
net_svc_sound_t sound;
- NET_SVC_Sound_Parse (&sound, net_message);
+ if (NET_SVC_Sound_Parse (&sound, net_message)) {
+ Host_NetError ("CL_ParseStartSoundPacket: Bad Read\n");
+ return;
+ }
- if (sound.entity > MAX_EDICTS)
+ if (sound.entity >= MAX_EDICTS) {
Host_NetError ("CL_ParseStartSoundPacket: ent = %i", sound.entity);
+ return;
+ }
+ if (sound.sound_num >= MAX_SOUNDS) {
+ Host_NetError ("CL_ParseStartSoundPacket: sound_num = %i",
+ sound.sound_num);
+ return;
+ }
S_StartSound (sound.entity, sound.channel,
cl.sound_precache[sound.sound_num], sound.position,
@@ -978,11 +1035,16 @@ CL_ParseUpdateUserInfo (void)
player_info_t *player;
net_svc_updateuserinfo_t updateuserinfo;
- NET_SVC_UpdateUserInfo_Parse (&updateuserinfo, net_message);
+ if (NET_SVC_UpdateUserInfo_Parse (&updateuserinfo, net_message)) {
+ Host_NetError ("CL_ParseUpdateUserInfo: Bad Read\n");
+ return;
+ }
- if (updateuserinfo.slot >= MAX_CLIENTS)
- Host_NetError ("CL_ParseServerMessage: svc_updateuserinfo > "
- "MAX_SCOREBOARD");
+ if (updateuserinfo.slot >= MAX_CLIENTS) {
+ Host_NetError ("CL_ParseUpdateUserInfo: slot %i >= MAX_CLIENTS",
+ updateuserinfo.slot);
+ return;
+ }
player = &cl.players[updateuserinfo.slot];
player->userid = updateuserinfo.userid;
@@ -999,10 +1061,15 @@ CL_SetInfo (void)
player_info_t *player;
net_svc_setinfo_t setinfo;
- NET_SVC_SetInfo_Parse (&setinfo, net_message);
+ if (NET_SVC_SetInfo_Parse (&setinfo, net_message)) {
+ Host_NetError ("CL_SetInfo: Bad Read\n");
+ return;
+ }
- if (setinfo.slot >= MAX_CLIENTS)
- Host_NetError ("CL_ParseServerMessage: svc_setinfo > MAX_SCOREBOARD");
+ if (setinfo.slot >= MAX_CLIENTS) {
+ Host_NetError ("CL_SetInfo: slot %i >= MAX_CLIENTS", setinfo.slot);
+ return;
+ }
player = &cl.players[setinfo.slot];
@@ -1022,7 +1089,10 @@ CL_ServerInfo (void)
{
net_svc_serverinfo_t block;
- NET_SVC_ServerInfo_Parse (&block, net_message);
+ if (NET_SVC_ServerInfo_Parse (&block, net_message)) {
+ Host_NetError ("CL_ServerInfo: Bad Read\n");
+ return;
+ }
Con_DPrintf ("SERVERINFO: %s=%s\n", block.key, block.value);
diff --git a/qw/source/cl_tent.c b/qw/source/cl_tent.c
index 9a20f32f9..4aaad4618 100644
--- a/qw/source/cl_tent.c
+++ b/qw/source/cl_tent.c
@@ -50,6 +50,7 @@ static const char rcsid[] =
#include "cl_main.h"
#include "cl_tent.h"
#include "client.h"
+#include "host.h"
#include "net_svc.h"
#include "r_dynamic.h"
@@ -188,6 +189,12 @@ CL_ParseBeam (net_svc_tempentity_t *tempentity, model_t *m)
beam_t *b;
int i;
+ if (tempentity->beamentity >= MAX_EDICTS) {
+ Host_NetError ("CL_ParseBeam: beamentity %i >= MAX_EDICTS",
+ tempentity->beamentity);
+ return;
+ }
+
// override any beam with the same entity
for (i = 0, b = cl_beams; i < MAX_BEAMS; i++, b++)
if (b->entity == tempentity->beamentity) {
@@ -222,7 +229,10 @@ CL_ParseTEnt (void)
int rnd;
net_svc_tempentity_t tempentity;
- NET_SVC_TempEntity_Parse (&tempentity, net_message);
+ if (NET_SVC_TempEntity_Parse (&tempentity, net_message)) {
+ Host_NetError ("CL_ParseTEnt: Bad Read\n");
+ return;
+ }
switch (tempentity.type) {
case TE_WIZSPIKE: // spike hitting wall
@@ -365,7 +375,7 @@ CL_ParseTEnt (void)
break;
default:
- Sys_Error ("CL_ParseTEnt: bad tempentity.type");
+ Sys_Error ("CL_ParseTEnt: bad tempentity.type %i", tempentity.type);
}
}
diff --git a/qw/source/cl_view.c b/qw/source/cl_view.c
index 40e3af38b..0cd742f1c 100644
--- a/qw/source/cl_view.c
+++ b/qw/source/cl_view.c
@@ -252,7 +252,10 @@ V_ParseDamage (void)
vec3_t forward, right, up;
net_svc_damage_t damage;
- NET_SVC_Damage_Parse (&damage, net_message);
+ if (NET_SVC_Damage_Parse (&damage, net_message)) {
+ Host_NetError ("V_ParseDamage: Bad Read\n");
+ return;
+ }
count = damage.blood * 0.5 + damage.armor * 0.5;
if (count < 10)
diff --git a/qw/source/net_svc.c b/qw/source/net_svc.c
index 9458ab64b..70afbde37 100644
--- a/qw/source/net_svc.c
+++ b/qw/source/net_svc.c
@@ -312,7 +312,8 @@ NET_SVC_Playerinfo_Parse (net_svc_playerinfo_t *block, msg_t *msg)
if (block->flags & PF_MODEL)
block->modelindex = MSG_ReadByte (msg);
- // FIXME: handle else
+ else
+ block->modelindex = 0; // suboptimal, but oh well
if (block->flags & PF_SKINNUM)
block->skinnum = MSG_ReadByte (msg);
@@ -357,6 +358,9 @@ NET_SVC_Nails_Parse (net_svc_nails_t *block, msg_t *msg)
block->nails[i].angles[2] = 0;
}
+ if (block->numnails > MAX_PROJECTILES)
+ block->numnails = MAX_PROJECTILES;
+
return msg->badread;
}