Skip to content

Commit

Permalink
More strictly check stat index bounds.
Browse files Browse the repository at this point in the history
Don't allow referencing stats >= 32 without protocol extensions.
  • Loading branch information
skullernet authored and res2k committed Jan 2, 2025
1 parent f234aee commit a17e39e
Show file tree
Hide file tree
Showing 9 changed files with 23 additions and 7 deletions.
6 changes: 3 additions & 3 deletions src/client/ascii.c
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ static void TH_DrawLayoutString(char *dst, const char *s)
width = Q_atoi(token);
token = COM_Parse(&s);
value = Q_atoi(token);
if (value < 0 || value >= MAX_STATS) {
if (value < 0 || value >= cl.max_stats) {
Com_Error(ERR_DROP, "%s: invalid stat index", __func__);
}
value = cl.frame.ps.stats[value];
Expand All @@ -244,7 +244,7 @@ static void TH_DrawLayoutString(char *dst, const char *s)
if (!strcmp(token, "stat_string")) {
token = COM_Parse(&s);
index = Q_atoi(token);
if (index < 0 || index >= MAX_STATS) {
if (index < 0 || index >= cl.max_stats) {
Com_Error(ERR_DROP, "%s: invalid string index", __func__);
}
index = cl.frame.ps.stats[index];
Expand Down Expand Up @@ -273,7 +273,7 @@ static void TH_DrawLayoutString(char *dst, const char *s)
if (!strcmp(token, "if")) {
token = COM_Parse(&s);
value = Q_atoi(token);
if (value < 0 || value >= MAX_STATS) {
if (value < 0 || value >= cl.max_stats) {
Com_Error(ERR_DROP, "%s: invalid stat index", __func__);
}
value = cl.frame.ps.stats[value];
Expand Down
8 changes: 7 additions & 1 deletion src/client/cgame.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ static bool CGX_IsExtendedServer(void)
return cl.csr.extended;
}

static int CGX_GetMaxStats(void)
{
return cl.max_stats;
}

static color_t apply_scr_alpha(color_t color)
{
color.a *= Cvar_ClampValue(scr_alpha, 0, 1);
Expand All @@ -51,9 +56,10 @@ static const pmoveParams_t* CGX_GetPmoveParams(void)
}

static cgame_q2pro_extended_support_ext_t cgame_q2pro_extended_support = {
.api_version = 2,
.api_version = 3,

.IsExtendedServer = CGX_IsExtendedServer,
.GetMaxStats = CGX_GetMaxStats,
.DrawCharEx = CGX_DrawCharEx,
.GetPmoveParams = CGX_GetPmoveParams,
};
Expand Down
4 changes: 3 additions & 1 deletion src/client/cgame_classic.c
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ bool SCR_ParseColor(const char *s, color_t *color);
static cgame_import_t cgi;
static cgame_q2pro_extended_support_ext_t cgix;
static const cs_remap_t *csr;
static int max_stats;

static cvar_t *scr_centertime;
static cvar_t *scr_draw2d;
Expand All @@ -67,6 +68,7 @@ static void CGC_Init(void)
/* We don't consider rerelease servers here and assume the appropriate
* cgame is used in that case */
csr = cgix.IsExtendedServer() ? &cs_remap_q2pro_new : &cs_remap_old;
max_stats = cgix.GetMaxStats();

scr_centertime = cgi.cvar("scr_centertime", "2.5", 0);
scr_draw2d = cgi.cvar("scr_draw2d", "2", 0);
Expand Down Expand Up @@ -288,7 +290,7 @@ static void layout_pic(vrect_t hud_vrect, const char **s, const player_state_t *
// draw a pic from a stat number
char* token = COM_Parse(s);
int value = atoi(token);
if (value < 0 || value >= MAX_STATS) {
if (value < 0 || value >= max_stats) {
cgi.Com_Error(va("%s: invalid stat index", __func__));
}
value = ps->stats[value];
Expand Down
3 changes: 3 additions & 0 deletions src/client/cgame_classic.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ typedef struct {
// Whether server is using "extended" protocol
bool (*IsExtendedServer)(void);

// Max allowed stats
int (*GetMaxStats)(void);

// Draw single character, colorized & w/ flags
void (*DrawCharEx)(int x, int y, int flags, int ch, color_t color);

Expand Down
1 change: 1 addition & 0 deletions src/client/client.h
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ typedef struct {
char gamedir[MAX_QPATH];
int clientNum; // never changed during gameplay, set by serverdata packet
int maxclients;
int max_stats;
pmoveParams_t pmp;

frametime_t frametime;
Expand Down
1 change: 1 addition & 0 deletions src/client/demo.c
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,7 @@ static void CL_PlayDemo_f(void)
Q_strlcpy(cls.servername, COM_SkipPath(name), sizeof(cls.servername));
cls.serverAddress.type = NA_LOOPBACK;
cl.csr = cs_remap_old;
cl.max_stats = MAX_STATS_OLD;

Con_Popup(true);
SCR_UpdateScreen();
Expand Down
1 change: 1 addition & 0 deletions src/client/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -1405,6 +1405,7 @@ static void CL_ConnectionlessPacket(void)
cls.connect_count = 0;
Q_strlcpy(cl.mapname, mapname, sizeof(cl.mapname)); // for levelshot screen
cl.csr = cs_remap_old;
cl.max_stats = MAX_STATS_OLD;
return;
}

Expand Down
2 changes: 2 additions & 0 deletions src/client/parse.c
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,8 @@ static void CL_ParseServerData(const q2proto_svc_serverdata_t *serverdata)
cls.demo.psFlags = cl.csr.extended ? CL_PS_EXTENDED_MASK_2 : 0;
}

cl.max_stats = (cl.game_api >= Q2PROTO_GAME_Q2PRO_EXTENDED_V2) ? MAX_STATS_NEW : MAX_STATS_OLD;

// Load cgame (after we know all the timings)
CG_Load(cl.gamedir, cl.game_api == Q2PROTO_GAME_RERELEASE);
cgame->Init();
Expand Down
4 changes: 2 additions & 2 deletions src/client/screen.c
Original file line number Diff line number Diff line change
Expand Up @@ -945,8 +945,8 @@ static void SCR_DrawDebugStats(void)
if (j <= 0)
return;

if (j > MAX_STATS)
j = MAX_STATS;
if (j > cl.max_stats)
j = cl.max_stats;

x = CONCHAR_WIDTH;
y = (scr.hud_height - j * CONCHAR_HEIGHT) / 2;
Expand Down

0 comments on commit a17e39e

Please sign in to comment.