From c4fdeeb4b036379cf34a19e952f2db3f5cef33cb Mon Sep 17 00:00:00 2001 From: NepDisk Date: Tue, 9 Dec 2025 20:37:41 -0500 Subject: [PATCH 1/5] Revert "Expand textcmd size byte to 16-bit word" This reverts commit 039a73ce6482b9724a20466f71acae6056fb690b. --- src/d_clisrv.c | 66 ++++++++++++++++++++++---------------------------- src/d_clisrv.h | 2 +- src/d_net.c | 17 +++---------- 3 files changed, 34 insertions(+), 51 deletions(-) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index 638a85ba4..c0d2cf52f 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -289,27 +289,27 @@ void RegisterNetXCmd(netxcmd_t id, void (*cmd_f)(UINT8 **p, INT32 playernum)) void SendNetXCmdForPlayer(UINT8 playerid, netxcmd_t id, const void *param, size_t nparam) { - if (((UINT16*)localtextcmd[playerid])[0]+3+nparam > MAXTEXTCMD) + if (localtextcmd[playerid][0]+2+nparam > MAXTEXTCMD) { // for future reference: if (cht_debug) != debug disabled. - CONS_Alert(CONS_ERROR, M_GetText("NetXCmd buffer full, cannot add netcmd %d! (size: %d, needed: %s)\n"), id, ((UINT16*)localtextcmd[playerid])[0], sizeu1(nparam)); + CONS_Alert(CONS_ERROR, M_GetText("NetXCmd buffer full, cannot add netcmd %d! (size: %d, needed: %s)\n"), id, localtextcmd[playerid][0], sizeu1(nparam)); return; } - ((UINT16*)localtextcmd[playerid])[0]++; - localtextcmd[playerid][((UINT16*)localtextcmd[playerid])[0] + 1] = (UINT8)id; + localtextcmd[playerid][0]++; + localtextcmd[playerid][localtextcmd[playerid][0]] = (UINT8)id; if (param && nparam) { - memcpy(&localtextcmd[playerid][((UINT16*)localtextcmd[playerid])[0] + 2], param, nparam); - ((UINT16*)localtextcmd[playerid])[0] = ((UINT16*)localtextcmd[playerid])[0] + (UINT8)nparam; + memcpy(&localtextcmd[playerid][localtextcmd[playerid][0] + 1], param, nparam); + localtextcmd[playerid][0] = (UINT8)(localtextcmd[playerid][0] + (UINT8)nparam); } } UINT8 GetFreeXCmdSize(UINT8 playerid) { - // -2 for the size and another -1 for the ID. - return (UINT8)(localtextcmd[playerid][0] - 3); + // -1 for the size and another -1 for the ID. + return (UINT8)(localtextcmd[playerid][0] - 2); } // Frees all textcmd memory for the specified tic @@ -422,9 +422,9 @@ static boolean ExtraDataTicker(void) if (bufferstart) { UINT8 *curpos = bufferstart; - UINT8 *bufferend = &curpos[((UINT16*)curpos)[0]+2]; + UINT8 *bufferend = &curpos[curpos[0]+1]; - curpos += 2; + curpos++; while (curpos < bufferend) { if (*curpos < MAXNETXCMD && listnetxcmd[*curpos]) @@ -4121,7 +4121,7 @@ void SV_StopServer(void) P_UnloadLevel(); for (i = 0; i < MAXSPLITSCREENPLAYERS; i++) - ((UINT16*)localtextcmd[i])[0] = 0; + localtextcmd[i][0] = 0; for (i = firstticstosend; i < firstticstosend + BACKUPTICS; i++) D_Clearticcmd(i); @@ -4173,7 +4173,7 @@ static size_t TotalTextCmdPerTic(tic_t tic) { UINT8 *textcmd = D_GetExistingTextcmd(tic, i); if ((!i || playeringame[i]) && textcmd) - total += 3 + ((UINT16*)textcmd)[0]; // "+2" for size and playernum + total += 2 + textcmd[0]; // "+2" for size and playernum } return total; @@ -4815,17 +4815,9 @@ static void PT_TextCmd(SINT8 node, INT32 netconsole) tic_t tic = maketic; UINT8 *textcmd; - UINT16 incoming_size; - - { - UINT8 *incoming = netbuffer->u.textcmd; - - incoming_size = READUINT16(incoming); - } - // ignore if the textcmd has a reported size of zero // this shouldn't be sent at all - if (!incoming_size) + if (!netbuffer->u.textcmd[0]) { DEBFILE(va("GetPacket: Textcmd with size 0 detected! (node %u, player %d)\n", node, netconsole)); @@ -4834,11 +4826,11 @@ static void PT_TextCmd(SINT8 node, INT32 netconsole) } // ignore if the textcmd size var is actually larger than it should be - // BASEPACKETSIZE + 2 (for size) + textcmd[0] should == datalength - if (incoming_size > (size_t)doomcom->datalength-BASEPACKETSIZE-2) + // BASEPACKETSIZE + 1 (for size) + textcmd[0] should == datalength + if (netbuffer->u.textcmd[0] > (size_t)doomcom->datalength-BASEPACKETSIZE-1) { DEBFILE(va("GetPacket: Bad Textcmd packet size! (expected %d, actual %s, node %u, player %d)\n", - incoming_size, sizeu1((size_t)doomcom->datalength-BASEPACKETSIZE-2), + netbuffer->u.textcmd[0], sizeu1((size_t)doomcom->datalength-BASEPACKETSIZE-1), node, netconsole)); Net_UnAcknowledgePacket(node); return; @@ -4847,12 +4839,12 @@ static void PT_TextCmd(SINT8 node, INT32 netconsole) // check if tic that we are making isn't too large else we cannot send it :( // doomcom->numslots+1 "+1" since doomcom->numslots can change within this time and sent time j = software_MAXPACKETLENGTH - - (incoming_size + 3 + BASESERVERTICSSIZE + - (netbuffer->u.textcmd[0]+2+BASESERVERTICSSIZE + (doomcom->numslots+1)*sizeof(ticcmd_t)); // search a tic that have enougth space in the ticcmd while ((textcmd = D_GetExistingTextcmd(tic, netconsole)), - (TotalTextCmdPerTic(tic) > j || incoming_size + (textcmd ? ((UINT16*)textcmd)[0] : 0) > MAXTEXTCMD) + (TotalTextCmdPerTic(tic) > j || netbuffer->u.textcmd[0] + (textcmd ? textcmd[0] : 0) > MAXTEXTCMD) && tic < firstticstosend + BACKUPTICS) tic++; @@ -4869,10 +4861,10 @@ static void PT_TextCmd(SINT8 node, INT32 netconsole) if (!textcmd) textcmd = D_GetTextcmd(tic, netconsole); DEBFILE(va("textcmd put in tic %u at position %d (player %d) ftts %u mk %u\n", - tic, ((UINT16*)textcmd)[0]+2, netconsole, firstticstosend, maketic)); + tic, textcmd[0]+1, netconsole, firstticstosend, maketic)); - memcpy(&textcmd[((UINT16*)textcmd)[0]+2], netbuffer->u.textcmd+2, incoming_size); - ((UINT16*)textcmd)[0] += incoming_size; + memcpy(&textcmd[textcmd[0]+1], netbuffer->u.textcmd+1, netbuffer->u.textcmd[0]); + textcmd[0] += (UINT8)netbuffer->u.textcmd[0]; } } @@ -5070,7 +5062,7 @@ static void PT_ServerTics(SINT8 node, INT32 netconsole) for (j = 0; j < numtxtpak; j++) { INT32 k = *txtpak++; // playernum - const size_t txtsize = ((UINT16*)txtpak)[0]+2; + const size_t txtsize = txtpak[0]+1; if (i >= gametic) // Don't copy old net commands memcpy(D_GetTextcmd(i, k), txtpak, txtsize); @@ -5761,7 +5753,7 @@ static void CL_SendClientCmd(void) // Send extra data if needed for (i = 0; i < MAXSPLITSCREENPLAYERS; i++) { - if (((UINT16*)localtextcmd[i])[0]) + if (localtextcmd[i][0]) { switch (i) { @@ -5779,10 +5771,10 @@ static void CL_SendClientCmd(void) break; } - memcpy(netbuffer->u.textcmd, localtextcmd[i], ((UINT16*)localtextcmd[i])[0]+2); + memcpy(netbuffer->u.textcmd, localtextcmd[i], localtextcmd[i][0]+1); // All extra data have been sent - if (HSendPacket(servernode, true, 0, ((UINT16*)localtextcmd[i])[0]+2)) // Send can fail... - ((UINT16*)localtextcmd[i])[0] = 0; + if (HSendPacket(servernode, true, 0, localtextcmd[i][0]+1)) // Send can fail... + localtextcmd[i][0] = 0; } } } @@ -5880,14 +5872,14 @@ static void SV_SendTics(void) for (j = 0; j < MAXPLAYERS; j++) { UINT8 *textcmd = D_GetExistingTextcmd(i, j); - INT32 size = textcmd ? ((UINT16*)textcmd)[0] : 0; + INT32 size = textcmd ? textcmd[0] : 0; if ((!j || playeringame[j]) && size) { (*ntextcmd)++; WRITEUINT8(bufpos, j); - WRITEUINT16(bufpos, ((UINT16*)textcmd)[0]); - WRITEMEM(bufpos, &textcmd[2], size); + memcpy(bufpos, textcmd, size + 1); + bufpos += size + 1; } } } diff --git a/src/d_clisrv.h b/src/d_clisrv.h index 9ea8b32bd..f08977d01 100644 --- a/src/d_clisrv.h +++ b/src/d_clisrv.h @@ -396,7 +396,7 @@ struct doomdata_t client4cmd_pak client4pak; servertics_pak serverpak; serverconfig_pak servercfg; - UINT8 textcmd[MAXTEXTCMD+2]; + UINT8 textcmd[MAXTEXTCMD+1]; char filetxpak[sizeof (filetx_pak)]; char fileack[sizeof (fileack_pak)]; UINT8 filereceived; diff --git a/src/d_net.c b/src/d_net.c index 58cab71d7..4e7c23191 100644 --- a/src/d_net.c +++ b/src/d_net.c @@ -884,20 +884,11 @@ static void DebugPrintpacket(const char *header) case PT_TEXTCMD: case PT_TEXTCMD2: case PT_TEXTCMD3: - case PT_TEXTCMD4: { - UINT16 size; - - { - UINT8 *p = netbuffer->u.textcmd; - - size = READUINT16(p); - } - - fprintf(debugfile, " length %d\n", size); - fprintf(debugfile, "[%s]", netxcmdnames[netbuffer->u.textcmd[2] - 1]); - fprintfstringnewline((char *)netbuffer->u.textcmd + 3, size - 2); + case PT_TEXTCMD4: + fprintf(debugfile, " length %d\n", netbuffer->u.textcmd[0]); + fprintf(debugfile, "[%s]", netxcmdnames[netbuffer->u.textcmd[1] - 1]); + fprintfstringnewline((char *)netbuffer->u.textcmd + 2, netbuffer->u.textcmd[0] - 1); break; - } case PT_SERVERCFG: fprintf(debugfile, " playerslots %d clientnode %d serverplayer %d " "gametic %u gamestate %d gametype %d modifiedgame %d\n", From 3d67382b0d9e86f15d04c052e443399662fca468 Mon Sep 17 00:00:00 2001 From: NepDisk Date: Tue, 9 Dec 2025 20:44:24 -0500 Subject: [PATCH 2/5] Just enough to get UBSAN in a netgame --- src/d_netcmd.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/d_netcmd.c b/src/d_netcmd.c index bb196fc1f..058054176 100644 --- a/src/d_netcmd.c +++ b/src/d_netcmd.c @@ -2140,6 +2140,8 @@ static void Got_NameAndColor(UINT8 **cp, INT32 playernum) kick = true; // availabilities + // TODO: Port commit from RR: 6d0637d3 + /* for (i = 0; i < MAXSKINS; i++) { UINT32 playerhasunlocked = (p->availabilities & (1 << i)); @@ -2165,6 +2167,7 @@ static void Got_NameAndColor(UINT8 **cp, INT32 playernum) break; } } + */ if (kick) { From 3e075e3e4e38779cc14c924ef0b5b1705704a4d9 Mon Sep 17 00:00:00 2001 From: NepDisk Date: Tue, 9 Dec 2025 21:20:49 -0500 Subject: [PATCH 3/5] Port various netxcmd changes from SRB2Classic based on commits: Lift NetXCmd limits Fix segfault when sending excessively large netcmds Simplify NetXCmd client distribution try fixing some crash with bogus netxcmds --- src/d_clisrv.c | 134 ++++++++++++++++++++++++++++++------------------- 1 file changed, 81 insertions(+), 53 deletions(-) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index c0d2cf52f..911285a79 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -188,20 +188,23 @@ tic_t firstconnectattempttime = 0; // Must be a power of two #define TEXTCMD_HASH_SIZE 4 -typedef struct textcmdplayer_s -{ - INT32 playernum; - UINT8 cmd[MAXTEXTCMD]; - struct textcmdplayer_s *next; -} textcmdplayer_t; - typedef struct textcmdtic_s { tic_t tic; - textcmdplayer_t *playercmds[TEXTCMD_HASH_SIZE]; + UINT8 *playercmds[MAXPLAYERS]; struct textcmdtic_s *next; } textcmdtic_t; +typedef struct textcmdbuf_s textcmdbuf_t; + +struct textcmdbuf_s +{ + textcmdbuf_t *next; + UINT8 cmd[MAXTEXTCMD]; +}; + +static textcmdbuf_t *textcmdbuf[MAXSPLITSCREENPLAYERS]; + static ticcmd_t playercmds[MAXPLAYERS]; ticcmd_t netcmds[BACKUPTICS][MAXPLAYERS]; static textcmdtic_t *textcmds[TEXTCMD_HASH_SIZE] = {NULL}; @@ -287,23 +290,57 @@ void RegisterNetXCmd(netxcmd_t id, void (*cmd_f)(UINT8 **p, INT32 playernum)) listnetxcmd[id] = cmd_f; } +static void WriteNetXCmd(UINT8 *cmd, netxcmd_t id, const void *param, size_t nparam) +{ + cmd[0]++; + cmd[cmd[0]] = (UINT8)id; + if (param && nparam) + { + memcpy(&cmd[cmd[0]+1], param, nparam); + cmd[0] = (UINT8)(cmd[0] + (UINT8)nparam); + } +} + void SendNetXCmdForPlayer(UINT8 playerid, netxcmd_t id, const void *param, size_t nparam) { if (localtextcmd[playerid][0]+2+nparam > MAXTEXTCMD) { - // for future reference: if (cht_debug) != debug disabled. - CONS_Alert(CONS_ERROR, M_GetText("NetXCmd buffer full, cannot add netcmd %d! (size: %d, needed: %s)\n"), id, localtextcmd[playerid][0], sizeu1(nparam)); + textcmdbuf_t *buf = textcmdbuf[playerid]; + + if (2+nparam > MAXTEXTCMD) + { + CONS_Alert(CONS_ERROR, M_GetText("packet too large to fit NetXCmd, cannot add netcmd %d! (size: %s, max: %d)\n"), id, sizeu1(2+nparam), MAXTEXTCMD); + return; + } + + // for future reference: if (cv_debug) != debug disabled. + CONS_Alert(CONS_NOTICE, M_GetText("NetXCmd buffer full, delaying netcmd %d... (size: %d, needed: %s)\n"), id, localtextcmd[playerid][0], sizeu1(nparam)); + if (buf == NULL) + { + textcmdbuf[playerid] = Z_Malloc(sizeof(textcmdbuf_t), PU_STATIC, NULL); + textcmdbuf[playerid]->cmd[0] = 0; + textcmdbuf[playerid]->next = NULL; + WriteNetXCmd(textcmdbuf[playerid]->cmd, id, param, nparam); + return; + } + + while (buf->next != NULL) + buf = buf->next; + + if (buf->cmd[0]+2+nparam > MAXTEXTCMD) + { + buf->next = Z_Malloc(sizeof(textcmdbuf_t), PU_STATIC, NULL); + buf->next->cmd[0] = 0; + buf->next->next = NULL; + WriteNetXCmd(buf->next->cmd, id, param, nparam); + } + else + { + WriteNetXCmd(buf->cmd, id, param, nparam); + } return; } - - localtextcmd[playerid][0]++; - localtextcmd[playerid][localtextcmd[playerid][0]] = (UINT8)id; - - if (param && nparam) - { - memcpy(&localtextcmd[playerid][localtextcmd[playerid][0] + 1], param, nparam); - localtextcmd[playerid][0] = (UINT8)(localtextcmd[playerid][0] + (UINT8)nparam); - } + WriteNetXCmd(localtextcmd[playerid], id, param, nparam); } UINT8 GetFreeXCmdSize(UINT8 playerid) @@ -326,22 +363,13 @@ static void D_FreeTextcmd(tic_t tic) if (textcmdtic) { - INT32 i; - // Remove this tic from the list. *tctprev = textcmdtic->next; // Free all players. - for (i = 0; i < TEXTCMD_HASH_SIZE; i++) + for (INT32 i = 0; i < MAXPLAYERS; i++) { - textcmdplayer_t *textcmdplayer = textcmdtic->playercmds[i]; - - while (textcmdplayer) - { - textcmdplayer_t *tcpnext = textcmdplayer->next; - Z_Free(textcmdplayer); - textcmdplayer = tcpnext; - } + Z_Free(textcmdtic->playercmds[i]); } // Free this tic's own memory. @@ -358,10 +386,8 @@ static UINT8* D_GetExistingTextcmd(tic_t tic, INT32 playernum) // Do we have an entry for the tic? If so, look for player. if (textcmdtic) { - textcmdplayer_t *textcmdplayer = textcmdtic->playercmds[playernum & (TEXTCMD_HASH_SIZE - 1)]; - while (textcmdplayer && textcmdplayer->playernum != playernum) textcmdplayer = textcmdplayer->next; - - if (textcmdplayer) return textcmdplayer->cmd; + UINT8 *cmd = textcmdtic->playercmds[playernum]; + if (cmd) return cmd; } return NULL; @@ -372,7 +398,6 @@ static UINT8* D_GetTextcmd(tic_t tic, INT32 playernum) { textcmdtic_t *textcmdtic = textcmds[tic & (TEXTCMD_HASH_SIZE - 1)]; textcmdtic_t **tctprev = &textcmds[tic & (TEXTCMD_HASH_SIZE - 1)]; - textcmdplayer_t *textcmdplayer, **tcpprev; // Look for the tic. while (textcmdtic && textcmdtic->tic != tic) @@ -388,24 +413,11 @@ static UINT8* D_GetTextcmd(tic_t tic, INT32 playernum) textcmdtic->tic = tic; } - tcpprev = &textcmdtic->playercmds[playernum & (TEXTCMD_HASH_SIZE - 1)]; - textcmdplayer = *tcpprev; - - // Look for the player. - while (textcmdplayer && textcmdplayer->playernum != playernum) - { - tcpprev = &textcmdplayer->next; - textcmdplayer = textcmdplayer->next; - } - // If we don't have an entry for the player, make it. - if (!textcmdplayer) - { - textcmdplayer = *tcpprev = Z_Calloc(sizeof (textcmdplayer_t), PU_STATIC, NULL); - textcmdplayer->playernum = playernum; - } + if (!textcmdtic->playercmds[playernum]) + textcmdtic->playercmds[playernum] = Z_Calloc(MAXTEXTCMD, PU_STATIC, NULL); - return textcmdplayer->cmd; + return textcmdtic->playercmds[playernum]; } static boolean ExtraDataTicker(void) @@ -5061,11 +5073,17 @@ static void PT_ServerTics(SINT8 node, INT32 netconsole) numtxtpak = *txtpak++; for (j = 0; j < numtxtpak; j++) { - INT32 k = *txtpak++; // playernum + INT32 playernum = *txtpak++; // playernum const size_t txtsize = txtpak[0]+1; + if (playernum < 0 || playernum >= MAXPLAYERS) + { + CONS_Alert(CONS_WARNING, "Got bogus NetXCmd packet targetting player %d\n", playernum); + return; + } + if (i >= gametic) // Don't copy old net commands - memcpy(D_GetTextcmd(i, k), txtpak, txtsize); + memcpy(D_GetTextcmd(i, playernum), txtpak, txtsize); txtpak += txtsize; } } @@ -5774,7 +5792,17 @@ static void CL_SendClientCmd(void) memcpy(netbuffer->u.textcmd, localtextcmd[i], localtextcmd[i][0]+1); // All extra data have been sent if (HSendPacket(servernode, true, 0, localtextcmd[i][0]+1)) // Send can fail... + { localtextcmd[i][0] = 0; + + if (textcmdbuf[i] != NULL) + { + textcmdbuf_t *buf = textcmdbuf[i]; + memcpy(localtextcmd[i], textcmdbuf[i]->cmd, textcmdbuf[i]->cmd[0]+1); + textcmdbuf[i] = textcmdbuf[i]->next; + Z_Free(buf); + } + } } } } From 531be32abde86d848e275b9fd925d5393de5a2d9 Mon Sep 17 00:00:00 2001 From: James R Date: Sun, 11 Feb 2024 15:40:04 -0800 Subject: [PATCH 4/5] Got_Luacmd: always read netxcmd data, even if command is not executed --- src/lua_consolelib.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/lua_consolelib.c b/src/lua_consolelib.c index 42edfb7c9..90ea7d292 100644 --- a/src/lua_consolelib.c +++ b/src/lua_consolelib.c @@ -33,8 +33,18 @@ static consvar_t *this_cvar; void Got_Luacmd(UINT8 **cp, INT32 playernum) { UINT8 i, argc, flags; + const char *argv[256]; char buf[256]; + argc = READUINT8(*cp); + argv[0] = (const char*)*cp; + SKIPSTRINGN(*cp, 255); + for (i = 1; i < argc; i++) + { + argv[i] = (const char*)*cp; + SKIPSTRINGN(*cp, 255); + } + // don't use I_Assert here, goto the deny code below // to clean up and kick people who try nefarious exploits // like sending random junk lua commands to crash the server @@ -47,8 +57,7 @@ void Got_Luacmd(UINT8 **cp, INT32 playernum) lua_getfield(gL, LUA_REGISTRYINDEX, "COM_Command"); // push COM_Command if (!lua_istable(gL, -1)) goto deny; - argc = READUINT8(*cp); - READSTRINGN(*cp, buf, 255); + strlcpy(buf, argv[0], 255); strlwr(buf); // must lowercase buffer lua_getfield(gL, -1, buf); // push command info table if (!lua_istable(gL, -1)) goto deny; @@ -77,7 +86,7 @@ void Got_Luacmd(UINT8 **cp, INT32 playernum) LUA_PushUserdata(gL, &players[playernum], META_PLAYER); for (i = 1; i < argc; i++) { - READSTRINGN(*cp, buf, 255); + strlcpy(buf, argv[i], 255); lua_pushstring(gL, buf); } LUA_Call(gL, (int)argc, 0, 1); // argc is 1-based, so this will cover the player we passed too. From 67e9fe3e49f375a76333ff75ae7660bc47fc813d Mon Sep 17 00:00:00 2001 From: NepDisk Date: Tue, 9 Dec 2025 21:46:39 -0500 Subject: [PATCH 5/5] Init textcmd buffer as NULL --- src/d_clisrv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/d_clisrv.c b/src/d_clisrv.c index 911285a79..7375a1233 100644 --- a/src/d_clisrv.c +++ b/src/d_clisrv.c @@ -203,7 +203,7 @@ struct textcmdbuf_s UINT8 cmd[MAXTEXTCMD]; }; -static textcmdbuf_t *textcmdbuf[MAXSPLITSCREENPLAYERS]; +static textcmdbuf_t *textcmdbuf[MAXSPLITSCREENPLAYERS] = {NULL}; static ticcmd_t playercmds[MAXPLAYERS]; ticcmd_t netcmds[BACKUPTICS][MAXPLAYERS];