From 653c5d07d4e3ab1929cbec49579e2942d0e0f5e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Mon, 4 Mar 2024 18:05:12 +0100 Subject: [PATCH] Add validation for `StrToInts` and `IntsToStr`, move and rename Rename `StrToInts` function to `str_to_int32` and add strict validation. Because this function should only used with trusted internal strings, assertions are added to ensure that no invalid UTF-8 is being encoded as integers and that the string is not truncated. Some buffer sizes are adjusted accordingly, so truncation cannot happen. Rename `IntsToStr` function to `int32_to_str` and add lenient validation. An additional argument specifying the size of the output buffer is added to assert that the size of the output buffer is sufficient. However, because this function is used to decode data sent by the server and read from maps and ghosts, invalid input data should never result in crashes or invalid UTF-8 strings. This function will replace invalid UTF-8 codepoints with `?` characters. The Unicode replacement character is not used, because it's 3 bytes long, so the string with added replacement characters may not fit into the buffer and this would also make the function more complicated, because it would require a temporary buffer. This provides reasonable support for loading old maps which might contain invalid UTF-8 in their layer, group or envelope names. The functions are renamed and moved from `gamecore.h` to `system.h`. The motivation for this is that the tools would otherwise have to depend on game-shared, which previously only worked because the functions were inline. The inline definition is not wanted, because it requires adding a `system.h` include in `gamecore.h`. Tests are added to ensure the function still behaves the same as before for valid inputs and correctly handles invalid inputs. --- src/base/system.cpp | 94 ++++++++++++++++ src/base/system.h | 3 + src/game/client/components/ghost.cpp | 10 +- src/game/client/gameclient.cpp | 6 +- src/game/client/gameclient.h | 2 +- src/game/editor/mapitems/map_io.cpp | 23 ++-- src/game/gamecore.h | 48 --------- src/game/server/player.cpp | 12 +-- src/game/server/teeinfo.cpp | 2 +- src/test/str.cpp | 155 +++++++++++++++++++++++++++ src/tools/demo_extract_chat.cpp | 7 +- src/tools/map_convert_07.cpp | 3 +- src/tools/map_diff.cpp | 5 +- src/tools/map_find_env.cpp | 1 - src/tools/map_replace_area.cpp | 1 - 15 files changed, 287 insertions(+), 85 deletions(-) diff --git a/src/base/system.cpp b/src/base/system.cpp index 4d5e5bc51..57aff5290 100644 --- a/src/base/system.cpp +++ b/src/base/system.cpp @@ -4037,6 +4037,100 @@ const char *str_next_token(const char *str, const char *delim, char *buffer, int return tok + len; } +void str_to_int32(int *ints, size_t num_ints, const char *str) +{ + dbg_assert(num_ints > 0, "str_to_int32: num_ints invalid"); + + // Clear all integers, which also ensures null-termination, + // as the last byte is never written to. + for(size_t i = 0; i < num_ints; i++) + { + ints[i] = 0; + } + + size_t byte_index = 0; + const size_t total_bytes = num_ints * sizeof(int) - 1; // -1 for null-termination + auto &&write_byte = [ints, &byte_index](int b) mutable { + ints[byte_index / sizeof(int)] |= b << ((sizeof(int) - byte_index % sizeof(int) - 1) * 8); + byte_index++; + }; + + // Write each UTF-8 codepoint individually + while(true) + { + const int codepoint = str_utf8_decode(&str); + dbg_assert(codepoint != -1, "str_to_int32: invalid UTF-8 in string"); + if(codepoint == 0) + { + break; + } + + char encoded[4]; + const size_t encoded_count = str_utf8_encode(encoded, codepoint); + dbg_assert(encoded_count <= total_bytes - byte_index, "str_to_int32: string truncated"); + for(size_t i = 0; i < encoded_count; i++) + { + write_byte(encoded[i] + 128); + } + } + + // Write padding + while(byte_index < total_bytes) + { + write_byte(128); + } +} + +void int32_to_str(const int *ints, size_t num_ints, char *str, size_t str_size) +{ + dbg_assert(num_ints > 0, "int32_to_str: num_ints invalid"); + dbg_assert(str_size >= num_ints * sizeof(int), "int32_to_str: str_size invalid"); + + // Unpack string without validation + size_t str_index = 0; + for(size_t int_index = 0; int_index < num_ints; int_index++) + { + const int current_int = ints[int_index]; + str[str_index] = ((current_int >> 24) & 0xff) - 128; + str_index++; + str[str_index] = ((current_int >> 16) & 0xff) - 128; + str_index++; + str[str_index] = ((current_int >> 8) & 0xff) - 128; + str_index++; + str[str_index] = (current_int & 0xff) - 128; + str_index++; + } + // Ensure null-termination + str[str_index - 1] = '\0'; + + // Validate + const char *check_str = str; + int str_check_index = 0; + while(true) + { + const char *prev_check_str = check_str; + const int codepoint = str_utf8_decode(&check_str); + if(codepoint == 0) + { + // Check for (early) null-termination. + str[str_check_index] = '\0'; + break; + } + const size_t codepoint_size = check_str - prev_check_str; + if(codepoint == -1) + { + // Replace invalid codepoints with question mark characters instead of the Unicode + // replacement character, because the replacement character is 3 bytes long, so the + // string with added replacement characters may not fit into the buffer. + for(size_t i = 0; i < codepoint_size; i++) + { + str[str_check_index + i] = '?'; + } + } + str_check_index += codepoint_size; + } +} + static_assert(sizeof(unsigned) == 4, "unsigned must be 4 bytes in size"); static_assert(sizeof(unsigned) == sizeof(int), "unsigned and int must have the same size"); diff --git a/src/base/system.h b/src/base/system.h index d7425a512..bc5b05500 100644 --- a/src/base/system.h +++ b/src/base/system.h @@ -2418,6 +2418,9 @@ const char *str_next_token(const char *str, const char *delim, char *buffer, int */ int str_in_list(const char *list, const char *delim, const char *needle); +void str_to_int32(int *ints, size_t num_ints, const char *str); +void int32_to_str(const int *ints, size_t num_ints, char *str, size_t str_size); + /** * Packs 4 big endian bytes into an unsigned. * diff --git a/src/game/client/components/ghost.cpp b/src/game/client/components/ghost.cpp index 87066d7fa..fa098cbd3 100644 --- a/src/game/client/components/ghost.cpp +++ b/src/game/client/components/ghost.cpp @@ -20,7 +20,7 @@ CGhost::CGhost() : void CGhost::GetGhostSkin(CGhostSkin *pSkin, const char *pSkinName, int UseCustomColor, int ColorBody, int ColorFeet) { - StrToInts(&pSkin->m_Skin0, 6, pSkinName); + str_to_int32(&pSkin->m_Skin0, 6, pSkinName); pSkin->m_UseCustomColor = UseCustomColor; pSkin->m_ColorBody = ColorBody; pSkin->m_ColorFeet = ColorFeet; @@ -385,8 +385,8 @@ void CGhost::OnRender() void CGhost::InitRenderInfos(CGhostItem *pGhost) { - char aSkinName[64]; - IntsToStr(&pGhost->m_Skin.m_Skin0, 6, aSkinName); + char aSkinName[24]; + int32_to_str(&pGhost->m_Skin.m_Skin0, 6, aSkinName, std::size(aSkinName)); CTeeRenderInfo *pRenderInfo = &pGhost->m_RenderInfo; const CSkin *pSkin = m_pClient->m_Skins.Find(aSkinName); @@ -689,8 +689,8 @@ void CGhost::OnRefreshSkins() const auto &&RefindSkin = [&](auto &Ghost) { if(Ghost.Empty()) return; - char aSkinName[64]; - IntsToStr(&Ghost.m_Skin.m_Skin0, 6, aSkinName); + char aSkinName[24]; + int32_to_str(&Ghost.m_Skin.m_Skin0, 6, aSkinName, std::size(aSkinName)); CTeeRenderInfo *pRenderInfo = &Ghost.m_RenderInfo; if(aSkinName[0] != '\0') { diff --git a/src/game/client/gameclient.cpp b/src/game/client/gameclient.cpp index 4f735c5b4..89e6f9cfd 100644 --- a/src/game/client/gameclient.cpp +++ b/src/game/client/gameclient.cpp @@ -1383,10 +1383,10 @@ void CGameClient::OnNewSnapshot() { CClientData *pClient = &m_aClients[ClientID]; - IntsToStr(&pInfo->m_Name0, 4, pClient->m_aName); - IntsToStr(&pInfo->m_Clan0, 3, pClient->m_aClan); + int32_to_str(&pInfo->m_Name0, 4, pClient->m_aName, std::size(pClient->m_aName)); + int32_to_str(&pInfo->m_Clan0, 3, pClient->m_aClan, std::size(pClient->m_aClan)); pClient->m_Country = pInfo->m_Country; - IntsToStr(&pInfo->m_Skin0, 6, pClient->m_aSkinName); + int32_to_str(&pInfo->m_Skin0, 6, pClient->m_aSkinName, std::size(pClient->m_aSkinName)); pClient->m_UseCustomColor = pInfo->m_UseCustomColor; pClient->m_ColorBody = pInfo->m_ColorBody; diff --git a/src/game/client/gameclient.h b/src/game/client/gameclient.h index c56a63ae0..a1342817f 100644 --- a/src/game/client/gameclient.h +++ b/src/game/client/gameclient.h @@ -358,7 +358,7 @@ public: char m_aName[MAX_NAME_LENGTH]; char m_aClan[MAX_CLAN_LENGTH]; int m_Country; - char m_aSkinName[64]; + char m_aSkinName[24]; int m_SkinColor; int m_Team; int m_Emoticon; diff --git a/src/game/editor/mapitems/map_io.cpp b/src/game/editor/mapitems/map_io.cpp index fa3c71dad..b0c17563c 100644 --- a/src/game/editor/mapitems/map_io.cpp +++ b/src/game/editor/mapitems/map_io.cpp @@ -8,7 +8,6 @@ #include #include -#include #include #include "image.h" @@ -174,7 +173,7 @@ bool CEditorMap::Save(const char *pFileName) GItem.m_NumLayers = 0; // save group name - StrToInts(GItem.m_aName, sizeof(GItem.m_aName) / sizeof(int), pGroup->m_aName); + str_to_int32(GItem.m_aName, std::size(GItem.m_aName), pGroup->m_aName); for(const std::shared_ptr &pLayer : pGroup->m_vpLayers) { @@ -243,7 +242,7 @@ bool CEditorMap::Save(const char *pFileName) Item.m_Data = Writer.AddData((size_t)pLayerTiles->m_Width * pLayerTiles->m_Height * sizeof(CTile), pLayerTiles->m_pTiles); // save layer name - StrToInts(Item.m_aName, sizeof(Item.m_aName) / sizeof(int), pLayerTiles->m_aName); + str_to_int32(Item.m_aName, std::size(Item.m_aName), pLayerTiles->m_aName); Writer.AddItem(MAPITEMTYPE_LAYER, LayerCount, sizeof(Item), &Item); @@ -285,7 +284,7 @@ bool CEditorMap::Save(const char *pFileName) Item.m_Data = Writer.AddDataSwapped(pLayerQuads->m_vQuads.size() * sizeof(CQuad), pLayerQuads->m_vQuads.data()); // save layer name - StrToInts(Item.m_aName, sizeof(Item.m_aName) / sizeof(int), pLayerQuads->m_aName); + str_to_int32(Item.m_aName, std::size(Item.m_aName), pLayerQuads->m_aName); Writer.AddItem(MAPITEMTYPE_LAYER, LayerCount, sizeof(Item), &Item); @@ -311,7 +310,7 @@ bool CEditorMap::Save(const char *pFileName) Item.m_Data = Writer.AddDataSwapped(pLayerSounds->m_vSources.size() * sizeof(CSoundSource), pLayerSounds->m_vSources.data()); // save layer name - StrToInts(Item.m_aName, sizeof(Item.m_aName) / sizeof(int), pLayerSounds->m_aName); + str_to_int32(Item.m_aName, std::size(Item.m_aName), pLayerSounds->m_aName); Writer.AddItem(MAPITEMTYPE_LAYER, LayerCount, sizeof(Item), &Item); GItem.m_NumLayers++; @@ -335,7 +334,7 @@ bool CEditorMap::Save(const char *pFileName) Item.m_StartPoint = PointCount; Item.m_NumPoints = m_vpEnvelopes[e]->m_vPoints.size(); Item.m_Synchronized = m_vpEnvelopes[e]->m_Synchronized; - StrToInts(Item.m_aName, sizeof(Item.m_aName) / sizeof(int), m_vpEnvelopes[e]->m_aName); + str_to_int32(Item.m_aName, std::size(Item.m_aName), m_vpEnvelopes[e]->m_aName); Writer.AddItem(MAPITEMTYPE_ENVELOPE, e, sizeof(Item), &Item); PointCount += Item.m_NumPoints; @@ -624,7 +623,7 @@ bool CEditorMap::Load(const char *pFileName, int StorageType, const std::functio // load group name if(pGItem->m_Version >= 3) - IntsToStr(pGItem->m_aName, sizeof(pGroup->m_aName) / sizeof(int), pGroup->m_aName); + int32_to_str(pGItem->m_aName, std::size(pGItem->m_aName), pGroup->m_aName, std::size(pGroup->m_aName)); for(int l = 0; l < pGItem->m_NumLayers; l++) { @@ -700,7 +699,7 @@ bool CEditorMap::Load(const char *pFileName, int StorageType, const std::functio // load layer name if(pTilemapItem->m_Version >= 3) - IntsToStr(pTilemapItem->m_aName, sizeof(pTiles->m_aName) / sizeof(int), pTiles->m_aName); + int32_to_str(pTilemapItem->m_aName, std::size(pTilemapItem->m_aName), pTiles->m_aName, std::size(pTiles->m_aName)); if(pTiles->m_Tele) { @@ -826,7 +825,7 @@ bool CEditorMap::Load(const char *pFileName, int StorageType, const std::functio // load layer name if(pQuadsItem->m_Version >= 2) - IntsToStr(pQuadsItem->m_aName, sizeof(pQuads->m_aName) / sizeof(int), pQuads->m_aName); + int32_to_str(pQuadsItem->m_aName, std::size(pQuadsItem->m_aName), pQuads->m_aName, std::size(pQuads->m_aName)); void *pData = DataFile.GetDataSwapped(pQuadsItem->m_Data); pGroup->AddLayer(pQuads); @@ -849,7 +848,7 @@ bool CEditorMap::Load(const char *pFileName, int StorageType, const std::functio pSounds->m_Sound = -1; // load layer name - IntsToStr(pSoundsItem->m_aName, sizeof(pSounds->m_aName) / sizeof(int), pSounds->m_aName); + int32_to_str(pSoundsItem->m_aName, std::size(pSoundsItem->m_aName), pSounds->m_aName, std::size(pSounds->m_aName)); // load data void *pData = DataFile.GetDataSwapped(pSoundsItem->m_Data); @@ -874,7 +873,7 @@ bool CEditorMap::Load(const char *pFileName, int StorageType, const std::functio pSounds->m_Sound = -1; // load layer name - IntsToStr(pSoundsItem->m_aName, sizeof(pSounds->m_aName) / sizeof(int), pSounds->m_aName); + int32_to_str(pSoundsItem->m_aName, std::size(pSoundsItem->m_aName), pSounds->m_aName, std::size(pSounds->m_aName)); // load data CSoundSource_DEPRECATED *pData = (CSoundSource_DEPRECATED *)DataFile.GetDataSwapped(pSoundsItem->m_Data); @@ -941,7 +940,7 @@ bool CEditorMap::Load(const char *pFileName, int StorageType, const std::functio mem_copy(&pEnv->m_vPoints[p].m_Bezier, pPointBezier, sizeof(CEnvPointBezier)); } if(pItem->m_aName[0] != -1) // compatibility with old maps - IntsToStr(pItem->m_aName, sizeof(pItem->m_aName) / sizeof(int), pEnv->m_aName); + int32_to_str(pItem->m_aName, std::size(pItem->m_aName), pEnv->m_aName, std::size(pEnv->m_aName)); m_vpEnvelopes.push_back(pEnv); if(pItem->m_Version >= CMapItemEnvelope_v2::CURRENT_VERSION) pEnv->m_Synchronized = pItem->m_Synchronized; diff --git a/src/game/gamecore.h b/src/game/gamecore.h index 8b16cfcaa..0e5baa067 100644 --- a/src/game/gamecore.h +++ b/src/game/gamecore.h @@ -65,54 +65,6 @@ public: float GetWeaponFireDelay(int Weapon) const; }; -inline void StrToInts(int *pInts, int Num, const char *pStr) -{ - int Index = 0; - while(Num) - { - char aBuf[4] = {0, 0, 0, 0}; -#ifdef __GNUC__ -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Warray-bounds" // false positive -#endif - for(int c = 0; c < 4 && pStr[Index]; c++, Index++) - aBuf[c] = pStr[Index]; -#ifdef __GNUC__ -#pragma GCC diagnostic pop -#endif - *pInts = ((aBuf[0] + 128) << 24) | ((aBuf[1] + 128) << 16) | ((aBuf[2] + 128) << 8) | (aBuf[3] + 128); - pInts++; - Num--; - } - - // null terminate - pInts[-1] &= 0xffffff00; -} - -inline void IntsToStr(const int *pInts, int Num, char *pStr) -{ - while(Num) - { - pStr[0] = (((*pInts) >> 24) & 0xff) - 128; - pStr[1] = (((*pInts) >> 16) & 0xff) - 128; - pStr[2] = (((*pInts) >> 8) & 0xff) - 128; - pStr[3] = ((*pInts) & 0xff) - 128; - pStr += 4; - pInts++; - Num--; - } - -#if defined(__GNUC__) && __GNUC__ >= 7 -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wstringop-overflow" // false positive -#endif - // null terminate - pStr[-1] = 0; -#if defined(__GNUC__) && __GNUC__ >= 7 -#pragma GCC diagnostic pop -#endif -} - inline vec2 CalcPos(vec2 Pos, vec2 Velocity, float Curvature, float Speed, float Time) { vec2 n; diff --git a/src/game/server/player.cpp b/src/game/server/player.cpp index 3ec8e7b44..deb1a90fa 100644 --- a/src/game/server/player.cpp +++ b/src/game/server/player.cpp @@ -315,10 +315,10 @@ void CPlayer::Snap(int SnappingClient) if(!pClientInfo) return; - StrToInts(&pClientInfo->m_Name0, 4, Server()->ClientName(m_ClientID)); - StrToInts(&pClientInfo->m_Clan0, 3, Server()->ClientClan(m_ClientID)); + str_to_int32(&pClientInfo->m_Name0, 4, Server()->ClientName(m_ClientID)); + str_to_int32(&pClientInfo->m_Clan0, 3, Server()->ClientClan(m_ClientID)); pClientInfo->m_Country = Server()->ClientCountry(m_ClientID); - StrToInts(&pClientInfo->m_Skin0, 6, m_TeeInfos.m_aSkinName); + str_to_int32(&pClientInfo->m_Skin0, 6, m_TeeInfos.m_aSkinName); pClientInfo->m_UseCustomColor = m_TeeInfos.m_UseCustomColor; pClientInfo->m_ColorBody = m_TeeInfos.m_ColorBody; pClientInfo->m_ColorFeet = m_TeeInfos.m_ColorFeet; @@ -465,9 +465,9 @@ void CPlayer::FakeSnap() if(!pClientInfo) return; - StrToInts(&pClientInfo->m_Name0, 4, " "); - StrToInts(&pClientInfo->m_Clan0, 3, ""); - StrToInts(&pClientInfo->m_Skin0, 6, "default"); + str_to_int32(&pClientInfo->m_Name0, 4, " "); + str_to_int32(&pClientInfo->m_Clan0, 3, ""); + str_to_int32(&pClientInfo->m_Skin0, 6, "default"); if(m_Paused != PAUSE_PAUSED) return; diff --git a/src/game/server/teeinfo.cpp b/src/game/server/teeinfo.cpp index fec3fe301..27b7f4d37 100644 --- a/src/game/server/teeinfo.cpp +++ b/src/game/server/teeinfo.cpp @@ -5,7 +5,7 @@ struct StdSkin { - char m_aSkinName[64]; + char m_aSkinName[24]; // body, marking, decoration, hands, feet, eyes char m_apSkinPartNames[6][24]; bool m_aUseCustomColors[6]; diff --git a/src/test/str.cpp b/src/test/str.cpp index a586e8110..158823409 100644 --- a/src/test/str.cpp +++ b/src/test/str.cpp @@ -1083,6 +1083,161 @@ TEST(Str, CountChar) EXPECT_EQ(str_countchr(pStr, 'y'), 0); } +TEST(Str, StrToInt32) +{ + int aInts[8]; + + str_to_int32(aInts, 1, "a"); + EXPECT_EQ(aInts[0], 0xE1808000); + + str_to_int32(aInts, 1, "ab"); + EXPECT_EQ(aInts[0], 0xE1E28000); + + str_to_int32(aInts, 1, "abc"); + EXPECT_EQ(aInts[0], 0xE1E2E300); + + str_to_int32(aInts, 2, "abcd"); + EXPECT_EQ(aInts[0], 0xE1E2E3E4); + EXPECT_EQ(aInts[1], 0x80808000); + + str_to_int32(aInts, 2, "abcde"); + EXPECT_EQ(aInts[0], 0xE1E2E3E4); + EXPECT_EQ(aInts[1], 0xE5808000); + + str_to_int32(aInts, 2, "abcdef"); + EXPECT_EQ(aInts[0], 0xE1E2E3E4); + EXPECT_EQ(aInts[1], 0xE5E68000); + + str_to_int32(aInts, 2, "abcdefg"); + EXPECT_EQ(aInts[0], 0xE1E2E3E4); + EXPECT_EQ(aInts[1], 0xE5E6E700); + + str_to_int32(aInts, 2, "öüä"); + EXPECT_EQ(aInts[0], 0x4336433C); + EXPECT_EQ(aInts[1], 0x43248000); + + str_to_int32(aInts, 3, "aβい🐘"); + EXPECT_EQ(aInts[0], 0xE14E3263); + EXPECT_EQ(aInts[1], 0x0104701F); + EXPECT_EQ(aInts[2], 0x10188000); + + // long padding + str_to_int32(aInts, 4, "abc"); + EXPECT_EQ(aInts[0], 0xE1E2E380); + EXPECT_EQ(aInts[1], 0x80808080); + EXPECT_EQ(aInts[2], 0x80808080); + EXPECT_EQ(aInts[3], 0x80808000); +} + +TEST(Str, Int32ToStr) +{ + int aInts[8]; + char aStr[sizeof(aInts)]; + + aInts[0] = 0xE1808000; + int32_to_str(aInts, 1, aStr, std::size(aStr)); + EXPECT_STREQ(aStr, "a"); + + aInts[0] = 0xE1E28000; + int32_to_str(aInts, 1, aStr, std::size(aStr)); + EXPECT_STREQ(aStr, "ab"); + + aInts[0] = 0xE1E2E300; + int32_to_str(aInts, 1, aStr, std::size(aStr)); + EXPECT_STREQ(aStr, "abc"); + + aInts[0] = 0xE1E2E3E4; + aInts[1] = 0x80808000; + int32_to_str(aInts, 2, aStr, std::size(aStr)); + EXPECT_STREQ(aStr, "abcd"); + + aInts[0] = 0xE1E2E3E4; + aInts[1] = 0xE5808000; + int32_to_str(aInts, 2, aStr, std::size(aStr)); + EXPECT_STREQ(aStr, "abcde"); + + aInts[0] = 0xE1E2E3E4; + aInts[1] = 0xE5E68000; + int32_to_str(aInts, 2, aStr, std::size(aStr)); + EXPECT_STREQ(aStr, "abcdef"); + + aInts[0] = 0xE1E2E3E4; + aInts[1] = 0xE5E6E700; + int32_to_str(aInts, 2, aStr, std::size(aStr)); + EXPECT_STREQ(aStr, "abcdefg"); + + aInts[0] = 0x4336433C; + aInts[1] = 0x43248000; + int32_to_str(aInts, 2, aStr, std::size(aStr)); + EXPECT_STREQ(aStr, "öüä"); + + aInts[0] = 0xE14E3263; + aInts[1] = 0x0104701F; + aInts[2] = 0x10188000; + int32_to_str(aInts, 3, aStr, std::size(aStr)); + EXPECT_STREQ(aStr, "aβい🐘"); + + // long padding + aInts[0] = 0xE1E2E380; + aInts[1] = 0x80808080; + aInts[2] = 0x80808080; + aInts[3] = 0x80808000; + int32_to_str(aInts, 4, aStr, std::size(aStr)); + EXPECT_STREQ(aStr, "abc"); + + // early null character (0x80) + aInts[0] = 0xE1E2E380; + aInts[1] = 0xE1E2E3E4; + aInts[2] = 0xE1E2E3E4; + aInts[3] = 0xE1E2E300; + int32_to_str(aInts, 4, aStr, std::size(aStr)); + EXPECT_STREQ(aStr, "abc"); + + // invalid UTF-8 + aInts[0] = 0xE17FE200; + int32_to_str(aInts, 1, aStr, std::size(aStr)); + EXPECT_STREQ(aStr, "a?b"); + + // invalid UTF-8 (0x00 in string data, which is translated to '\x80') + aInts[0] = 0xE100E200; + int32_to_str(aInts, 1, aStr, std::size(aStr)); + EXPECT_STREQ(aStr, "a?b"); + + // invalid UTF-8 + aInts[0] = 0xE1E2E36F; + aInts[1] = 0x3F40E4E5; + aInts[2] = 0xE67FE700; + int32_to_str(aInts, 3, aStr, std::size(aStr)); + EXPECT_STREQ(aStr, "abc???def?g"); + + // invalid UTF-8 and missing null-terminator + aInts[0] = 0x7F7F7F7F; + int32_to_str(aInts, 1, aStr, std::size(aStr)); + EXPECT_STREQ(aStr, "???"); + + // missing null-terminator at the end is ignored + aInts[0] = 0xE1E2E3E4; + int32_to_str(aInts, 1, aStr, std::size(aStr)); + EXPECT_STREQ(aStr, "abc"); + + // basic fuzzing: no input integer should result in invalid UTF-8 in the string + for(int i = 0; i <= 0xFFFFFF; i += 7) // checking all values takes a bit too long + { + mem_zero(aStr, std::size(aStr)); + aInts[0] = 0xE1 << 24 | i; + aInts[1] = i << 8 | 0xE2; + aInts[2] = 0xE3 << 24 | i; + int32_to_str(aInts, 3, aStr, std::size(aStr)); + // the valid codespoints should always appear at the expected positions + ASSERT_EQ(aStr[0], 'a'); + ASSERT_EQ(aStr[7], 'b'); + ASSERT_EQ(aStr[8], 'c'); + // ensure null-termination before calling str_utf8_check + ASSERT_EQ(aStr[11], '\0'); + ASSERT_TRUE(str_utf8_check(aStr)); + } +} + #if defined(CONF_FAMILY_WINDOWS) TEST(Str, WindowsUtf8WideConversion) { diff --git a/src/tools/demo_extract_chat.cpp b/src/tools/demo_extract_chat.cpp index 82f1879e3..c101b163f 100644 --- a/src/tools/demo_extract_chat.cpp +++ b/src/tools/demo_extract_chat.cpp @@ -7,8 +7,6 @@ #include #include -#include - static const char *TOOL_NAME = "demo_extract_chat"; class CClientSnapshotHandler @@ -103,8 +101,13 @@ public: int ClientID = Item.m_ID; if(ClientID < MAX_CLIENTS) { +<<<<<<< HEAD CClientData *pClient = &m_aClients[ClientID]; IntsToStr(&pInfo->m_Name0, 4, pClient->m_aName); +======= + CClientData *pClient = &m_aClients[ClientId]; + int32_to_str(&pInfo->m_Name0, 4, pClient->m_aName, sizeof(pClient->m_aName)); +>>>>>>> 1138e763b (Add validation for `StrToInts` and `IntsToStr`, move and rename) } } } diff --git a/src/tools/map_convert_07.cpp b/src/tools/map_convert_07.cpp index 699701658..d64b41518 100644 --- a/src/tools/map_convert_07.cpp +++ b/src/tools/map_convert_07.cpp @@ -7,7 +7,6 @@ #include #include #include -#include #include /* Usage: map_convert_07 @@ -96,7 +95,7 @@ bool CheckImageDimensions(void *pLayerItem, int LayerType, const char *pFilename return true; char aTileLayerName[12]; - IntsToStr(pTMap->m_aName, sizeof(pTMap->m_aName) / sizeof(int), aTileLayerName); + int32_to_str(pTMap->m_aName, std::size(pTMap->m_aName), aTileLayerName, std::size(aTileLayerName)); const char *pName = g_DataReader.GetDataString(pImgItem->m_ImageName); dbg_msg("map_convert_07", "%s: Tile layer \"%s\" uses image \"%s\" with width %d, height %d, which is not divisible by 16. This is not supported in Teeworlds 0.7. Please scale the image and replace it manually.", pFilename, aTileLayerName, pName == nullptr ? "(error)" : pName, pImgItem->m_Width, pImgItem->m_Height); diff --git a/src/tools/map_diff.cpp b/src/tools/map_diff.cpp index 799d330bf..719e17cf2 100644 --- a/src/tools/map_diff.cpp +++ b/src/tools/map_diff.cpp @@ -2,7 +2,6 @@ #include #include #include -#include #include bool Process(IStorage *pStorage, const char **pMapNames) @@ -60,12 +59,12 @@ bool Process(IStorage *pStorage, const char **pMapNames) continue; CMapItemLayerTilemap *apTilemap[2]; - char aaName[2][16]; + char aaName[2][12]; for(int i = 0; i < 2; ++i) { apTilemap[i] = (CMapItemLayerTilemap *)apItem[i]; - IntsToStr(apTilemap[i]->m_aName, sizeof(apTilemap[i]->m_aName) / sizeof(int), aaName[i]); + int32_to_str(apTilemap[i]->m_aName, std::size(apTilemap[i]->m_aName), aaName[i], std::size(aaName[i])); } if(str_comp(aaName[0], aaName[1]) != 0 || apTilemap[0]->m_Width != apTilemap[1]->m_Width || apTilemap[0]->m_Height != apTilemap[1]->m_Height) diff --git a/src/tools/map_find_env.cpp b/src/tools/map_find_env.cpp index db5ed7c95..e0b620c60 100644 --- a/src/tools/map_find_env.cpp +++ b/src/tools/map_find_env.cpp @@ -2,7 +2,6 @@ #include #include #include -#include #include struct EnvelopedQuad diff --git a/src/tools/map_replace_area.cpp b/src/tools/map_replace_area.cpp index ab963897c..3e3952a17 100644 --- a/src/tools/map_replace_area.cpp +++ b/src/tools/map_replace_area.cpp @@ -2,7 +2,6 @@ #include #include #include -#include #include // global new layers data (set by ReplaceAreaTiles and ReplaceAreaQuads)