Merge branch 'fix-playernode-crash' into 'next'

Fix buffer overrun with players with no associated node

See merge request STJr/SRB2!810
This commit is contained in:
LJ Sonic 2020-03-15 15:36:45 -04:00
commit 0a54c40ff4
4 changed files with 65 additions and 56 deletions

View File

@ -1289,6 +1289,37 @@ static boolean CL_SendJoin(void)
return HSendPacket(servernode, true, 0, sizeof (clientconfig_pak));
}
static INT32 FindRejoinerNum(SINT8 node)
{
char strippednodeaddress[64];
const char *nodeaddress;
char *port;
INT32 i;
// Make sure there is no dead dress before proceeding to the stripping
if (!I_GetNodeAddress)
return -1;
nodeaddress = I_GetNodeAddress(node);
if (!nodeaddress)
return -1;
// Strip the address of its port
strcpy(strippednodeaddress, nodeaddress);
port = strchr(strippednodeaddress, ':');
if (port)
*port = '\0';
// Check if any player matches the stripped address
for (i = 0; i < MAXPLAYERS; i++)
{
if (playeringame[i] && playeraddress[i][0] && playernode[i] == UINT8_MAX
&& !strcmp(playeraddress[i], strippednodeaddress))
return i;
}
return -1;
}
static void SV_SendServerInfo(INT32 node, tic_t servertime)
{
UINT8 *p;
@ -1306,6 +1337,16 @@ static void SV_SendServerInfo(INT32 node, tic_t servertime)
netbuffer->u.serverinfo.numberofplayer = (UINT8)D_NumPlayers();
netbuffer->u.serverinfo.maxplayer = (UINT8)cv_maxplayers.value;
if (FindRejoinerNum(node) != -1)
netbuffer->u.serverinfo.refusereason = 0;
else if (!cv_allownewplayer.value)
netbuffer->u.serverinfo.refusereason = 1;
else if (D_NumPlayers() >= cv_maxplayers.value)
netbuffer->u.serverinfo.refusereason = 2;
else
netbuffer->u.serverinfo.refusereason = 0;
strncpy(netbuffer->u.serverinfo.gametypename, Gametype_Names[gametype],
sizeof netbuffer->u.serverinfo.gametypename);
netbuffer->u.serverinfo.modifiedgame = (UINT8)modifiedgame;
@ -1863,12 +1904,17 @@ static boolean CL_ServerConnectionSearchTicker(boolean viams, tic_t *asksent)
}
// Quit here rather than downloading files and being refused later.
if (serverlist[i].info.numberofplayer >= serverlist[i].info.maxplayer)
if (serverlist[i].info.refusereason)
{
D_QuitNetGame();
CL_Reset();
D_StartTitle();
M_StartMessage(va(M_GetText("Maximum players reached: %d\n\nPress ESC\n"), serverlist[i].info.maxplayer), NULL, MM_NOTHING);
if (serverlist[i].info.refusereason == 1)
M_StartMessage(M_GetText("The server is not accepting\njoins for the moment.\n\nPress ESC\n"), NULL, MM_NOTHING);
else if (serverlist[i].info.refusereason == 2)
M_StartMessage(va(M_GetText("Maximum players reached: %d\n\nPress ESC\n"), serverlist[i].info.maxplayer), NULL, MM_NOTHING);
else
M_StartMessage(M_GetText("You can't join.\nI don't know why,\nbut you can't join.\n\nPress ESC\n"), NULL, MM_NOTHING);
return false;
}
@ -2439,14 +2485,14 @@ static void CL_RemovePlayer(INT32 playernum, kickreason_t reason)
if (!playeringame[playernum])
return;
if (server && !demoplayback)
if (server && !demoplayback && playernode[playernum] != UINT8_MAX)
{
INT32 node = playernode[playernum];
playerpernode[node]--;
if (playerpernode[node] <= 0)
{
nodeingame[playernode[playernum]] = false;
Net_CloseConnection(playernode[playernum]);
nodeingame[node] = false;
Net_CloseConnection(node);
ResetNode(node);
}
}
@ -2792,16 +2838,13 @@ static void Command_Kick(void)
if (pn == -1 || pn == 0)
return;
if (server)
// Special case if we are trying to kick a player who is downloading the game state:
// trigger a timeout instead of kicking them, because a kick would only
// take effect after they have finished downloading
if (server && playernode[pn] != UINT8_MAX && sendingsavegame[playernode[pn]])
{
// Special case if we are trying to kick a player who is downloading the game state:
// trigger a timeout instead of kicking them, because a kick would only
// take effect after they have finished downloading
if (sendingsavegame[playernode[pn]])
{
Net_ConnectionTimeout(playernode[pn]);
return;
}
Net_ConnectionTimeout(playernode[pn]);
return;
}
WRITESINT8(p, pn);
@ -2859,7 +2902,7 @@ static void Got_KickCmd(UINT8 **p, INT32 playernum)
// Is playernum authorized to make this kick?
if (playernum != serverplayer && !IsPlayerAdmin(playernum)
&& !(playerpernode[playernode[playernum]] == 2
&& !(playernode[playernum] != UINT8_MAX && playerpernode[playernode[playernum]] == 2
&& nodetoplayer2[playernode[playernum]] == pnum))
{
// We received a kick command from someone who isn't the
@ -3018,7 +3061,7 @@ static void Got_KickCmd(UINT8 **p, INT32 playernum)
}
else if (keepbody)
{
if (server && !demoplayback)
if (server && !demoplayback && playernode[pnum] != UINT8_MAX)
{
INT32 node = playernode[pnum];
playerpernode[node]--;
@ -3234,37 +3277,6 @@ void D_QuitNetGame(void)
#endif
}
static INT32 FindRejoinerNum(SINT8 node)
{
char strippednodeaddress[64];
const char *nodeaddress;
char *port;
INT32 i;
// Make sure there is no dead dress before proceeding to the stripping
if (!I_GetNodeAddress)
return -1;
nodeaddress = I_GetNodeAddress(node);
if (!nodeaddress)
return -1;
// Strip the address of its port
strcpy(strippednodeaddress, nodeaddress);
port = strchr(strippednodeaddress, ':');
if (port)
*port = '\0';
// Check if any player matches the stripped address
for (i = 0; i < MAXPLAYERS; i++)
{
if (playeringame[i] && playeraddress[i][0] && playernode[i] == UINT8_MAX
&& !strcmp(playeraddress[i], strippednodeaddress))
return i;
}
return -1;
}
// Adds a node to the game (player will follow at map change or at savegame....)
static inline void SV_AddNode(INT32 node)
{
@ -3595,7 +3607,7 @@ static void HandleConnect(SINT8 node)
rejoinernum = FindRejoinerNum(node);
if (bannednode && bannednode[node])
SV_SendRefuse(node, M_GetText("You have been banned\nfrom the server"));
SV_SendRefuse(node, M_GetText("You have been banned\nfrom the server."));
else if (netbuffer->u.clientcfg._255 != 255 ||
netbuffer->u.clientcfg.packetversion != PACKETVERSION)
SV_SendRefuse(node, "Incompatible packet formats.");
@ -3606,7 +3618,7 @@ static void HandleConnect(SINT8 node)
|| netbuffer->u.clientcfg.subversion != SUBVERSION)
SV_SendRefuse(node, va(M_GetText("Different SRB2 versions cannot\nplay a netgame!\n(server version %d.%d.%d)"), VERSION/100, VERSION%100, SUBVERSION));
else if (!cv_allownewplayer.value && node && rejoinernum == -1)
SV_SendRefuse(node, M_GetText("The server is not accepting\njoins for the moment"));
SV_SendRefuse(node, M_GetText("The server is not accepting\njoins for the moment."));
else if (D_NumPlayers() >= cv_maxplayers.value && rejoinernum == -1)
SV_SendRefuse(node, va(M_GetText("Maximum players reached: %d"), cv_maxplayers.value));
else if (netgame && netbuffer->u.clientcfg.localplayers > 1) // Hacked client?
@ -4971,7 +4983,7 @@ void NetUpdate(void)
PingUpdate();
// update node latency values so we can take an average later.
for (i = 0; i < MAXPLAYERS; i++)
if (playeringame[i])
if (playeringame[i] && playernode[i] != UINT8_MAX)
realpingtable[i] += G_TicsToMilliseconds(GetLag(playernode[i]));
pingmeasurecount++;
}

View File

@ -27,7 +27,7 @@ This version is independent of the mod name, and standard
version and subversion. It should only account for the
basic fields of the packet, and change infrequently.
*/
#define PACKETVERSION 2
#define PACKETVERSION 3
// Network play related stuff.
// There is a data struct that stores network
@ -367,6 +367,7 @@ typedef struct
UINT8 subversion;
UINT8 numberofplayer;
UINT8 maxplayer;
UINT8 refusereason; // 0: joinable, 1: joins disabled, 2: full
char gametypename[24];
UINT8 modifiedgame;
UINT8 cheatsenabled;

View File

@ -3344,10 +3344,6 @@ static void Got_RequestAddfilecmd(UINT8 **cp, INT32 playernum)
boolean kick = false;
boolean toomany = false;
INT32 i,j;
serverinfo_pak *dummycheck = NULL;
// Shut the compiler up.
(void)dummycheck;
READSTRINGN(*cp, filename, 240);
READMEM(*cp, md5sum, 16);

View File

@ -10394,7 +10394,7 @@ static void M_DrawConnectMenu(void)
for (i = 0; i < min(serverlistcount - serverlistpage * SERVERS_PER_PAGE, SERVERS_PER_PAGE); i++)
{
INT32 slindex = i + serverlistpage * SERVERS_PER_PAGE;
UINT32 globalflags = ((serverlist[slindex].info.numberofplayer >= serverlist[slindex].info.maxplayer) ? V_TRANSLUCENT : 0)
UINT32 globalflags = (serverlist[slindex].info.refusereason ? V_TRANSLUCENT : 0)
|((itemOn == FIRSTSERVERLINE+i) ? V_YELLOWMAP : 0)|V_ALLOWLOWERCASE;
V_DrawString(currentMenu->x, S_LINEY(i), globalflags, serverlist[slindex].info.servername);