Merge pull request #8073 from Robyt3/Gamecore-Str-Validation

Add validation for `StrToInts` and `IntsToStr`
This commit is contained in:
heinrich5991 2024-03-10 13:48:08 +00:00 committed by GitHub
commit 3805ffba86
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 236 additions and 73 deletions

View file

@ -2707,6 +2707,7 @@ if(TOOLS)
src/tools/${TOOL}.cpp
${EXTRA_TOOL_SRC}
$<TARGET_OBJECTS:engine-shared>
$<TARGET_OBJECTS:game-shared>
)
target_include_directories(${TOOL} SYSTEM PRIVATE ${TOOL_INCLUDE_DIRS})
target_link_libraries(${TOOL} ${TOOL_LIBS})

View file

@ -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];
IntsToStr(&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];
IntsToStr(&Ghost.m_Skin.m_Skin0, 6, aSkinName, std::size(aSkinName));
CTeeRenderInfo *pRenderInfo = &Ghost.m_RenderInfo;
if(aSkinName[0] != '\0')
{

View file

@ -1364,10 +1364,13 @@ void CGameClient::OnNewSnapshot()
{
CClientData *pClient = &m_aClients[ClientId];
IntsToStr(&pInfo->m_Name0, 4, pClient->m_aName);
IntsToStr(&pInfo->m_Clan0, 3, pClient->m_aClan);
if(!IntsToStr(&pInfo->m_Name0, 4, pClient->m_aName, std::size(pClient->m_aName)))
{
str_copy(pClient->m_aName, "nameless tee");
}
IntsToStr(&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);
IntsToStr(&pInfo->m_Skin0, 6, pClient->m_aSkinName, std::size(pClient->m_aSkinName));
pClient->m_UseCustomColor = pInfo->m_UseCustomColor;
pClient->m_ColorBody = pInfo->m_ColorBody;

View file

@ -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;

View file

@ -174,7 +174,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);
StrToInts(GItem.m_aName, std::size(GItem.m_aName), pGroup->m_aName);
for(const std::shared_ptr<CLayer> &pLayer : pGroup->m_vpLayers)
{
@ -243,7 +243,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);
StrToInts(Item.m_aName, std::size(Item.m_aName), pLayerTiles->m_aName);
Writer.AddItem(MAPITEMTYPE_LAYER, LayerCount, sizeof(Item), &Item);
@ -285,7 +285,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);
StrToInts(Item.m_aName, std::size(Item.m_aName), pLayerQuads->m_aName);
Writer.AddItem(MAPITEMTYPE_LAYER, LayerCount, sizeof(Item), &Item);
@ -311,7 +311,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);
StrToInts(Item.m_aName, std::size(Item.m_aName), pLayerSounds->m_aName);
Writer.AddItem(MAPITEMTYPE_LAYER, LayerCount, sizeof(Item), &Item);
GItem.m_NumLayers++;
@ -335,7 +335,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);
StrToInts(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 +624,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);
IntsToStr(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 +700,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);
IntsToStr(pTilemapItem->m_aName, std::size(pTilemapItem->m_aName), pTiles->m_aName, std::size(pTiles->m_aName));
if(pTiles->m_Tele)
{
@ -826,7 +826,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);
IntsToStr(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 +849,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);
IntsToStr(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 +874,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);
IntsToStr(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 +941,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);
IntsToStr(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;

View file

@ -63,6 +63,57 @@ float CTuningParams::GetWeaponFireDelay(int Weapon) const
}
}
void StrToInts(int *pInts, size_t NumInts, const char *pStr)
{
dbg_assert(NumInts > 0, "StrToInts: NumInts invalid");
const size_t StrSize = str_length(pStr) + 1;
dbg_assert(StrSize <= NumInts * sizeof(int), "StrToInts: string truncated");
for(size_t i = 0; i < NumInts; i++)
{
// Copy to temporary buffer to ensure we don't read past the end of the input string
char aBuf[sizeof(int)] = {0, 0, 0, 0};
for(size_t c = 0; c < sizeof(int) && i * sizeof(int) + c < StrSize; c++)
{
aBuf[c] = pStr[i * sizeof(int) + c];
}
pInts[i] = ((aBuf[0] + 128) << 24) | ((aBuf[1] + 128) << 16) | ((aBuf[2] + 128) << 8) | (aBuf[3] + 128);
}
// Last byte is always zero and unused in this format
pInts[NumInts - 1] &= 0xFFFFFF00;
}
bool IntsToStr(const int *pInts, size_t NumInts, char *pStr, size_t StrSize)
{
dbg_assert(NumInts > 0, "IntsToStr: NumInts invalid");
dbg_assert(StrSize >= NumInts * sizeof(int), "IntsToStr: StrSize invalid");
// Unpack string without validation
size_t StrIndex = 0;
for(size_t IntIndex = 0; IntIndex < NumInts; IntIndex++)
{
const int CurrentInt = pInts[IntIndex];
pStr[StrIndex] = ((CurrentInt >> 24) & 0xff) - 128;
StrIndex++;
pStr[StrIndex] = ((CurrentInt >> 16) & 0xff) - 128;
StrIndex++;
pStr[StrIndex] = ((CurrentInt >> 8) & 0xff) - 128;
StrIndex++;
pStr[StrIndex] = (CurrentInt & 0xff) - 128;
StrIndex++;
}
// Ensure null-termination
pStr[StrIndex - 1] = '\0';
// Ensure valid UTF-8
if(str_utf8_check(pStr))
{
return true;
}
pStr[0] = '\0';
return false;
}
float VelocityRamp(float Value, float Start, float Range, float Curvature)
{
if(Value < Start)

View file

@ -65,53 +65,9 @@ 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
}
// Do not use these function unless for legacy code!
void StrToInts(int *pInts, size_t NumInts, const char *pStr);
bool IntsToStr(const int *pInts, size_t NumInts, char *pStr, size_t StrSize);
inline vec2 CalcPos(vec2 Pos, vec2 Velocity, float Curvature, float Speed, float Time)
{

View file

@ -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];

View file

@ -2,6 +2,8 @@
#include <base/system.h>
#include <game/gamecore.h>
TEST(Str, Dist)
{
EXPECT_EQ(str_utf8_dist("aaa", "aaa"), 0);
@ -1083,6 +1085,158 @@ TEST(Str, CountChar)
EXPECT_EQ(str_countchr(pStr, 'y'), 0);
}
TEST(Str, StrToInts)
{
int aInts[8];
StrToInts(aInts, 1, "a");
EXPECT_EQ(aInts[0], 0xE1808000);
StrToInts(aInts, 1, "ab");
EXPECT_EQ(aInts[0], 0xE1E28000);
StrToInts(aInts, 1, "abc");
EXPECT_EQ(aInts[0], 0xE1E2E300);
StrToInts(aInts, 2, "abcd");
EXPECT_EQ(aInts[0], 0xE1E2E3E4);
EXPECT_EQ(aInts[1], 0x80808000);
StrToInts(aInts, 2, "abcde");
EXPECT_EQ(aInts[0], 0xE1E2E3E4);
EXPECT_EQ(aInts[1], 0xE5808000);
StrToInts(aInts, 2, "abcdef");
EXPECT_EQ(aInts[0], 0xE1E2E3E4);
EXPECT_EQ(aInts[1], 0xE5E68000);
StrToInts(aInts, 2, "abcdefg");
EXPECT_EQ(aInts[0], 0xE1E2E3E4);
EXPECT_EQ(aInts[1], 0xE5E6E700);
StrToInts(aInts, 2, "öüä");
EXPECT_EQ(aInts[0], 0x4336433C);
EXPECT_EQ(aInts[1], 0x43248000);
StrToInts(aInts, 3, "aβい🐘");
EXPECT_EQ(aInts[0], 0xE14E3263);
EXPECT_EQ(aInts[1], 0x0104701F);
EXPECT_EQ(aInts[2], 0x10188000);
// long padding
StrToInts(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, IntsToStr)
{
int aInts[8];
char aStr[sizeof(aInts)];
aInts[0] = 0xE1808000;
EXPECT_TRUE(IntsToStr(aInts, 1, aStr, std::size(aStr)));
EXPECT_STREQ(aStr, "a");
aInts[0] = 0xE1E28000;
EXPECT_TRUE(IntsToStr(aInts, 1, aStr, std::size(aStr)));
EXPECT_STREQ(aStr, "ab");
aInts[0] = 0xE1E2E300;
EXPECT_TRUE(IntsToStr(aInts, 1, aStr, std::size(aStr)));
EXPECT_STREQ(aStr, "abc");
aInts[0] = 0xE1E2E3E4;
aInts[1] = 0x80808000;
EXPECT_TRUE(IntsToStr(aInts, 2, aStr, std::size(aStr)));
EXPECT_STREQ(aStr, "abcd");
aInts[0] = 0xE1E2E3E4;
aInts[1] = 0xE5808000;
EXPECT_TRUE(IntsToStr(aInts, 2, aStr, std::size(aStr)));
EXPECT_STREQ(aStr, "abcde");
aInts[0] = 0xE1E2E3E4;
aInts[1] = 0xE5E68000;
EXPECT_TRUE(IntsToStr(aInts, 2, aStr, std::size(aStr)));
EXPECT_STREQ(aStr, "abcdef");
aInts[0] = 0xE1E2E3E4;
aInts[1] = 0xE5E6E700;
EXPECT_TRUE(IntsToStr(aInts, 2, aStr, std::size(aStr)));
EXPECT_STREQ(aStr, "abcdefg");
aInts[0] = 0x4336433C;
aInts[1] = 0x43248000;
EXPECT_TRUE(IntsToStr(aInts, 2, aStr, std::size(aStr)));
EXPECT_STREQ(aStr, "öüä");
aInts[0] = 0xE14E3263;
aInts[1] = 0x0104701F;
aInts[2] = 0x10188000;
EXPECT_TRUE(IntsToStr(aInts, 3, aStr, std::size(aStr)));
EXPECT_STREQ(aStr, "aβい🐘");
// long padding
aInts[0] = 0xE1E2E380;
aInts[1] = 0x80808080;
aInts[2] = 0x80808080;
aInts[3] = 0x80808000;
EXPECT_TRUE(IntsToStr(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;
EXPECT_TRUE(IntsToStr(aInts, 4, aStr, std::size(aStr)));
EXPECT_STREQ(aStr, "abc");
// invalid UTF-8
aInts[0] = 0xE17FE200;
EXPECT_FALSE(IntsToStr(aInts, 1, aStr, std::size(aStr)));
EXPECT_STREQ(aStr, "");
// invalid UTF-8 (0x00 in string data, which is translated to '\x80')
aInts[0] = 0xE100E200;
EXPECT_FALSE(IntsToStr(aInts, 1, aStr, std::size(aStr)));
EXPECT_STREQ(aStr, "");
// invalid UTF-8
aInts[0] = 0xE1E2E36F;
aInts[1] = 0x3F40E4E5;
aInts[2] = 0xE67FE700;
EXPECT_FALSE(IntsToStr(aInts, 3, aStr, std::size(aStr)));
EXPECT_STREQ(aStr, "");
// invalid UTF-8 and missing null-terminator
aInts[0] = 0x7F7F7F7F;
EXPECT_FALSE(IntsToStr(aInts, 1, aStr, std::size(aStr)));
EXPECT_STREQ(aStr, "");
// missing null-terminator at the end is ignored
aInts[0] = 0xE1E2E3E4;
EXPECT_TRUE(IntsToStr(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;
const bool ConversionResult = IntsToStr(aInts, 3, aStr, std::size(aStr));
// ensure null-termination before calling str_utf8_check
ASSERT_TRUE(mem_has_null(aStr, std::size(aStr)));
ASSERT_TRUE(str_utf8_check(aStr));
ASSERT_TRUE(ConversionResult || aStr[0] == '\0');
}
}
#if defined(CONF_FAMILY_WINDOWS)
TEST(Str, WindowsUtf8WideConversion)
{

View file

@ -104,7 +104,7 @@ public:
if(ClientId < MAX_CLIENTS)
{
CClientData *pClient = &m_aClients[ClientId];
IntsToStr(&pInfo->m_Name0, 4, pClient->m_aName);
IntsToStr(&pInfo->m_Name0, 4, pClient->m_aName, sizeof(pClient->m_aName));
}
}
}

View file

@ -96,7 +96,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);
IntsToStr(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);

View file

@ -60,12 +60,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]);
IntsToStr(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)

View file

@ -2,7 +2,6 @@
#include <base/system.h>
#include <engine/shared/datafile.h>
#include <engine/storage.h>
#include <game/gamecore.h>
#include <game/mapitems.h>
struct EnvelopedQuad

View file

@ -2,7 +2,6 @@
#include <base/system.h>
#include <engine/shared/datafile.h>
#include <engine/storage.h>
#include <game/gamecore.h>
#include <game/mapitems.h>
// global new layers data (set by ReplaceAreaTiles and ReplaceAreaQuads)