From 3b52c955b18667bd6ea319f89e762c5331603bc9 Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Sun, 26 Mar 2023 09:27:26 +0200 Subject: [PATCH 45/51] - made D_WriteUserInfoStrings memory safe. Its callers are anything but for now but this function was the main blocker for refactoring so it had to come first. --- src/d_net.cpp | 8 +++- src/d_netinf.h | 2 +- src/d_netinfo.cpp | 106 +++++++++++++++++++++++----------------------- src/g_game.cpp | 10 +++-- 4 files changed, 65 insertions(+), 61 deletions(-) diff --git a/src/d_net.cpp b/src/d_net.cpp index e83b06939..80f43e1a2 100644 --- a/src/d_net.cpp +++ b/src/d_net.cpp @@ -1505,7 +1505,9 @@ bool DoArbitrate (void *userdata) netbuffer[1] = consoleplayer; netbuffer[9] = data->gotsetup[0]; stream = &netbuffer[10]; - D_WriteUserInfoStrings (consoleplayer, &stream, true); + auto str = D_GetUserInfoStrings (consoleplayer, true); + memcpy(stream, str.GetChars(), str.Len() + 1); + stream += str.Len(); SendSetup (data->playersdetected, data->gotsetup, int(stream - netbuffer)); } else @@ -1520,7 +1522,9 @@ bool DoArbitrate (void *userdata) { netbuffer[1] = j; stream = &netbuffer[9]; - D_WriteUserInfoStrings (j, &stream, true); + auto str = D_GetUserInfoStrings(j, true); + memcpy(stream, str.GetChars(), str.Len() + 1); + stream += str.Len(); HSendPacket (i, int(stream - netbuffer)); } } diff --git a/src/d_netinf.h b/src/d_netinf.h index 5e46b7b14..56f955e06 100644 --- a/src/d_netinf.h +++ b/src/d_netinf.h @@ -58,7 +58,7 @@ bool D_SendServerInfoChange (FBaseCVar *cvar, UCVarValue value, ECVarType type); bool D_SendServerFlagChange (FBaseCVar *cvar, int bitnum, bool set, bool silent); void D_DoServerInfoChange (uint8_t **stream, bool singlebit); -void D_WriteUserInfoStrings (int player, uint8_t **stream, bool compact=false); +FString D_GetUserInfoStrings(int pnum, bool compact = false); void D_ReadUserInfoStrings (int player, uint8_t **stream, bool update); struct FPlayerColorSet; diff --git a/src/d_netinfo.cpp b/src/d_netinfo.cpp index 6c31fbe73..bbcd42168 100644 --- a/src/d_netinfo.cpp +++ b/src/d_netinfo.cpp @@ -727,67 +727,65 @@ static int namesortfunc(const void *a, const void *b) return stricmp(name1->GetChars(), name2->GetChars()); } -void D_WriteUserInfoStrings (int pnum, uint8_t **stream, bool compact) +FString D_GetUserInfoStrings(int pnum, bool compact) { - if (pnum >= MAXPLAYERS) + FString result; + if (pnum >= 0 && pnum < MAXPLAYERS) { - WriteByte (0, stream); - return; - } - - userinfo_t *info = &players[pnum].userinfo; - TArray::Pair *> userinfo_pairs(info->CountUsed()); - TMap::Iterator it(*info); - TMap::Pair *pair; - UCVarValue cval; + userinfo_t* info = &players[pnum].userinfo; + TArray::Pair*> userinfo_pairs(info->CountUsed()); + TMap::Iterator it(*info); + TMap::Pair* pair; + UCVarValue cval; - // Create a simple array of all userinfo cvars - while (it.NextPair(pair)) - { - userinfo_pairs.Push(pair); - } - // For compact mode, these need to be sorted. Verbose mode doesn't matter. - if (compact) - { - qsort(&userinfo_pairs[0], userinfo_pairs.Size(), sizeof(pair), userinfosortfunc); - // Compact mode is signified by starting the string with two backslash characters. - // We output one now. The second will be output as part of the first value. - *(*stream)++ = '\\'; - } - for (unsigned int i = 0; i < userinfo_pairs.Size(); ++i) - { - pair = userinfo_pairs[i]; - - if (!compact) - { // In verbose mode, prepend the cvar's name - *stream += sprintf(*((char **)stream), "\\%s", pair->Key.GetChars()); + // Create a simple array of all userinfo cvars + while (it.NextPair(pair)) + { + userinfo_pairs.Push(pair); + } + // For compact mode, these need to be sorted. Verbose mode doesn't matter. + if (compact) + { + qsort(&userinfo_pairs[0], userinfo_pairs.Size(), sizeof(pair), userinfosortfunc); + // Compact mode is signified by starting the string with two backslash characters. + // We output one now. The second will be output as part of the first value. + result += '\\'; } - // A few of these need special handling for compatibility reasons. - switch (pair->Key.GetIndex()) + for (unsigned int i = 0; i < userinfo_pairs.Size(); ++i) { - case NAME_Gender: - *stream += sprintf(*((char **)stream), "\\%s", - *static_cast(pair->Value) == GENDER_FEMALE ? "female" : - *static_cast(pair->Value) == GENDER_NEUTER ? "neutral" : - *static_cast(pair->Value) == GENDER_OBJECT ? "other" : "male"); - break; - - case NAME_PlayerClass: - *stream += sprintf(*((char **)stream), "\\%s", info->GetPlayerClassNum() == -1 ? "Random" : - D_EscapeUserInfo(info->GetPlayerClassType()->GetDisplayName().GetChars()).GetChars()); - break; - - case NAME_Skin: - *stream += sprintf(*((char **)stream), "\\%s", D_EscapeUserInfo(Skins[info->GetSkin()].Name).GetChars()); - break; - - default: - cval = pair->Value->GetGenericRep(CVAR_String); - *stream += sprintf(*((char **)stream), "\\%s", cval.String); - break; + pair = userinfo_pairs[i]; + + if (!compact) + { // In verbose mode, prepend the cvar's name + result.AppendFormat("\\%s", pair->Key.GetChars()); + } + // A few of these need special handling for compatibility reasons. + switch (pair->Key.GetIndex()) + { + case NAME_Gender: + result.AppendFormat("\\%s", + *static_cast(pair->Value) == GENDER_FEMALE ? "female" : + *static_cast(pair->Value) == GENDER_NEUTER ? "neutral" : + *static_cast(pair->Value) == GENDER_OBJECT ? "other" : "male"); + break; + + case NAME_PlayerClass: + result.AppendFormat("\\%s", info->GetPlayerClassNum() == -1 ? "Random" : + D_EscapeUserInfo(info->GetPlayerClassType()->GetDisplayName().GetChars()).GetChars()); + break; + + case NAME_Skin: + result.AppendFormat("\\%s", D_EscapeUserInfo(Skins[info->GetSkin()].Name).GetChars()); + break; + + default: + cval = pair->Value->GetGenericRep(CVAR_String); + result.AppendFormat("\\%s", cval.String); + break; + } } } - *(*stream)++ = '\0'; + return result; } void D_ReadUserInfoStrings (int pnum, uint8_t **stream, bool update) diff --git a/src/g_game.cpp b/src/g_game.cpp index c960bab26..7ac7fd98d 100644 --- a/src/g_game.cpp +++ b/src/g_game.cpp @@ -2618,10 +2618,12 @@ void G_BeginRecording (const char *startmap) { if (playeringame[i]) { - StartChunk (UINF_ID, &demo_p); - WriteByte ((uint8_t)i, &demo_p); - D_WriteUserInfoStrings (i, &demo_p); - FinishChunk (&demo_p); + StartChunk(UINF_ID, &demo_p); + WriteByte((uint8_t)i, &demo_p); + auto str = D_GetUserInfoStrings(i); + memcpy(demo_p, str.GetChars(), str.Len() + 1); + demo_p += str.Len(); + FinishChunk(&demo_p); } } -- 2.39.3