From af9164f336a65634f6908e677b47773d88f583af Mon Sep 17 00:00:00 2001 From: Louis-Antoine Date: Sat, 22 Feb 2020 02:17:21 +0100 Subject: [PATCH 1/3] Improve join refusal handling --- src/d_clisrv.c | 81 ++++++++++++++++++++++++++++++-------------------- src/d_clisrv.h | 3 +- src/m_menu.c | 2 +- 3 files changed, 51 insertions(+), 35 deletions(-) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index 77118110e..ef0c0c33e 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -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; } @@ -3230,37 +3276,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) { diff --git a/src/d_clisrv.h b/src/d_clisrv.h index df93fe31d..8cea2f00a 100644 --- a/src/d_clisrv.h +++ b/src/d_clisrv.h @@ -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 @@ -361,6 +361,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; diff --git a/src/m_menu.c b/src/m_menu.c index 8b564e068..d48714c35 100644 --- a/src/m_menu.c +++ b/src/m_menu.c @@ -10366,7 +10366,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); From 778715f6111a1bbf7d2ae874e5490e1b2ae0dbc1 Mon Sep 17 00:00:00 2001 From: Louis-Antoine Date: Sat, 22 Feb 2020 02:21:27 +0100 Subject: [PATCH 2/3] Cleanup code a little --- src/d_clisrv.c | 4 ++-- src/d_netcmd.c | 4 ---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index ef0c0c33e..98eff1cd5 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -3606,7 +3606,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."); @@ -3617,7 +3617,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? diff --git a/src/d_netcmd.c b/src/d_netcmd.c index 623a83c47..b619621c6 100644 --- a/src/d_netcmd.c +++ b/src/d_netcmd.c @@ -3342,10 +3342,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); From d19fe295e0556a49c3f5c37df1e2b379a961611e Mon Sep 17 00:00:00 2001 From: Louis-Antoine Date: Sat, 22 Feb 2020 15:08:17 +0100 Subject: [PATCH 3/3] Fix buffer overrun with nodeless players --- src/d_clisrv.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index 98eff1cd5..eda6f9c67 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -2485,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); } } @@ -2838,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); @@ -2905,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 @@ -3064,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]--; @@ -4956,7 +4953,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++; }