Fix client crash on maps with invalid image and sound indices

Limit number of sounds being loaded in the client to 64, instead of writing sample indices out of bounds.

Check if sound indices are out of bounds. It was only checked whether the sound index is -1 but no other bounds checking was performed.

Check if image indices are out of bounds. It was not checked for all negative image indices, only -1 was considered.

Add popup message in the editor when trying to add more than 64 sounds, same as for images.

Limit number of images in `map_convert_07` tool, instead of writing out of bounds.

src/game/client/components/mapimages.cpp

@@ -68,7 +68,7 @@ void CMapImages::OnMapLoadImpl(class CLayers *pLayers, IMap *pMap)
 	int Start;
 	pMap->GetType(MAPITEMTYPE_IMAGE, &Start, &m_Count);
 
-	m_Count = clamp(m_Count, 0, 64);
+	m_Count = clamp<int>(m_Count, 0, MAX_MAPIMAGES);
 
 	for(int g = 0; g < pLayers->NumGroups(); g++)
 	{
@@ -84,7 +84,7 @@ void CMapImages::OnMapLoadImpl(class CLayers *pLayers, IMap *pMap)
 			if(pLayer->m_Type == LAYERTYPE_TILES)
 			{
 				CMapItemLayerTilemap *pTLayer = (CMapItemLayerTilemap *)pLayer;
-				if(pTLayer->m_Image != -1 && pTLayer->m_Image < (int)(std::size(m_aTextures)))
+				if(pTLayer->m_Image >= 0 && pTLayer->m_Image < m_Count)
 				{
 					m_aTextureUsedByTileOrQuadLayerFlag[pTLayer->m_Image] |= 1;
 				}
@@ -92,7 +92,7 @@ void CMapImages::OnMapLoadImpl(class CLayers *pLayers, IMap *pMap)
 			else if(pLayer->m_Type == LAYERTYPE_QUADS)
 			{
 				CMapItemLayerQuads *pQLayer = (CMapItemLayerQuads *)pLayer;
-				if(pQLayer->m_Image != -1 && pQLayer->m_Image < (int)(std::size(m_aTextures)))
+				if(pQLayer->m_Image >= 0 && pQLayer->m_Image < m_Count)
 				{
 					m_aTextureUsedByTileOrQuadLayerFlag[pQLayer->m_Image] |= 2;
 				}

src/game/client/components/mapimages.h

@@ -6,6 +6,7 @@
 #include <engine/graphics.h>
 
 #include <game/client/component.h>
+#include <game/mapitems.h>
 
 enum EMapImageEntityLayerType
 {
@@ -35,8 +36,8 @@ class CMapImages : public CComponent
 	friend class CBackground;
 	friend class CMenuBackground;
 
-	IGraphics::CTextureHandle m_aTextures[64];
+	IGraphics::CTextureHandle m_aTextures[MAX_MAPIMAGES];
-	int m_aTextureUsedByTileOrQuadLayerFlag[64]; // 0: nothing, 1(as flag): tile layer, 2(as flag): quad layer
+	int m_aTextureUsedByTileOrQuadLayerFlag[MAX_MAPIMAGES]; // 0: nothing, 1(as flag): tile layer, 2(as flag): quad layer
 	int m_Count;
 
 	char m_aEntitiesPath[IO_MAX_PATH_LENGTH];

src/game/client/components/maplayers.cpp

@@ -1739,7 +1739,7 @@ void CMapLayers::OnRender()
 				if(pLayer->m_Type == LAYERTYPE_TILES)
 				{
 					CMapItemLayerTilemap *pTMap = (CMapItemLayerTilemap *)pLayer;
-					if(pTMap->m_Image == -1)
+					if(pTMap->m_Image < 0 || pTMap->m_Image >= m_pImages->Num())
 					{
 						if(!IsGameLayer)
 							Graphics()->TextureClear();
@@ -1803,7 +1803,7 @@ void CMapLayers::OnRender()
 				else if(pLayer->m_Type == LAYERTYPE_QUADS)
 				{
 					CMapItemLayerQuads *pQLayer = (CMapItemLayerQuads *)pLayer;
-					if(pQLayer->m_Image == -1)
+					if(pQLayer->m_Image < 0 || pQLayer->m_Image >= m_pImages->Num())
 						Graphics()->TextureClear();
 					else
 						Graphics()->TextureSet(m_pImages->Get(pQLayer->m_Image));

src/game/client/components/mapsounds.cpp

@@ -26,6 +26,8 @@ void CMapSounds::OnMapLoad()
 	int Start;
 	pMap->GetType(MAPITEMTYPE_SOUND, &Start, &m_Count);
 
+	m_Count = clamp<int>(m_Count, 0, MAX_MAPSOUNDS);
+
 	// load new samples
 	for(int i = 0; i < m_Count; i++)
 	{
@@ -85,15 +87,14 @@ void CMapSounds::OnMapLoad()
 
 				for(int i = 0; i < pSoundLayer->m_NumSources; i++)
 				{
-					CSourceQueueEntry source;
+					CSourceQueueEntry Source;
-					source.m_Sound = pSoundLayer->m_Sound;
+					Source.m_Sound = pSoundLayer->m_Sound;
-					source.m_pSource = &pSources[i];
+					Source.m_pSource = &pSources[i];
-					source.m_HighDetail = pLayer->m_Flags & LAYERFLAG_DETAIL;
 
-					if(!source.m_pSource || source.m_Sound == -1)
+					if(!Source.m_pSource || Source.m_Sound < 0 || Source.m_Sound >= m_Count)
 						continue;
 
-					m_vSourceQueue.push_back(source);
+					m_vSourceQueue.push_back(Source);
 				}
 			}
 		}

src/game/client/components/mapsounds.h

@@ -6,18 +6,18 @@
 #include <engine/sound.h>
 
 #include <game/client/component.h>
+#include <game/mapitems.h>
 
 struct CSoundSource;
 
 class CMapSounds : public CComponent
 {
-	int m_aSounds[64];
+	int m_aSounds[MAX_MAPSOUNDS];
 	int m_Count;
 
 	struct CSourceQueueEntry
 	{
 		int m_Sound;
-		bool m_HighDetail;
 		ISound::CVoiceHandle m_Voice;
 		CSoundSource *m_pSource;
 

src/game/editor/editor.cpp

@@ -3978,7 +3978,7 @@ bool CEditor::AddImage(const char *pFileName, int StorageType, void *pUser)
 		}
 	}
 
-	if(pEditor->m_Map.m_vpImages.size() >= 64) // hard limit for teeworlds
+	if(pEditor->m_Map.m_vpImages.size() >= MAX_MAPIMAGES)
 	{
 		pEditor->m_PopupEventType = POPEVENT_IMAGE_MAX;
 		pEditor->m_PopupEventActivated = true;
@@ -4039,6 +4039,13 @@ bool CEditor::AddSound(const char *pFileName, int StorageType, void *pUser)
 		}
 	}
 
+	if(pEditor->m_Map.m_vpSounds.size() >= MAX_MAPSOUNDS)
+	{
+		pEditor->m_PopupEventType = POPEVENT_SOUND_MAX;
+		pEditor->m_PopupEventActivated = true;
+		return false;
+	}
+
 	// load external
 	void *pData;
 	unsigned DataSize;

src/game/editor/editor.h

@@ -523,6 +523,7 @@ public:
 		POPEVENT_PREVENTUNUSEDTILES,
 		POPEVENT_IMAGEDIV16,
 		POPEVENT_IMAGE_MAX,
+		POPEVENT_SOUND_MAX,
 		POPEVENT_PLACE_BORDER_TILES,
 		POPEVENT_PIXELART_BIG_IMAGE,
 		POPEVENT_PIXELART_MANY_COLORS,

src/game/editor/popups.cpp

@@ -1864,6 +1864,7 @@ CUI::EPopupMenuFunctionResult CEditor::PopupEvent(void *pContext, CUIRect View,
 
 	const char *pTitle;
 	const char *pMessage;
+	char aMessageBuf[128];
 	if(pEditor->m_PopupEventType == POPEVENT_EXIT)
 	{
 		pTitle = "Exit the editor";
@@ -1912,7 +1913,14 @@ CUI::EPopupMenuFunctionResult CEditor::PopupEvent(void *pContext, CUIRect View,
 	else if(pEditor->m_PopupEventType == POPEVENT_IMAGE_MAX)
 	{
 		pTitle = "Max images";
-		pMessage = "The client only allows a maximum of 64 images.";
+		str_format(aMessageBuf, sizeof(aMessageBuf), "The client only allows a maximum of %" PRIzu " images.", MAX_MAPIMAGES);
+		pMessage = aMessageBuf;
+	}
+	else if(pEditor->m_PopupEventType == POPEVENT_SOUND_MAX)
+	{
+		pTitle = "Max sounds";
+		str_format(aMessageBuf, sizeof(aMessageBuf), "The client only allows a maximum of %" PRIzu " sounds.", MAX_MAPSOUNDS);
+		pMessage = aMessageBuf;
 	}
 	else if(pEditor->m_PopupEventType == POPEVENT_PLACE_BORDER_TILES)
 	{
@@ -1956,7 +1964,12 @@ CUI::EPopupMenuFunctionResult CEditor::PopupEvent(void *pContext, CUIRect View,
 
 	// button bar
 	ButtonBar.VSplitLeft(110.0f, &Button, &ButtonBar);
-	if(pEditor->m_PopupEventType != POPEVENT_LARGELAYER && pEditor->m_PopupEventType != POPEVENT_PREVENTUNUSEDTILES && pEditor->m_PopupEventType != POPEVENT_IMAGEDIV16 && pEditor->m_PopupEventType != POPEVENT_IMAGE_MAX && pEditor->m_PopupEventType != POPEVENT_PIXELART_TOO_MANY_COLORS)
+	if(pEditor->m_PopupEventType != POPEVENT_LARGELAYER &&
+		pEditor->m_PopupEventType != POPEVENT_PREVENTUNUSEDTILES &&
+		pEditor->m_PopupEventType != POPEVENT_IMAGEDIV16 &&
+		pEditor->m_PopupEventType != POPEVENT_IMAGE_MAX &&
+		pEditor->m_PopupEventType != POPEVENT_SOUND_MAX &&
+		pEditor->m_PopupEventType != POPEVENT_PIXELART_TOO_MANY_COLORS)
 	{
 		static int s_CancelButton = 0;
 		if(pEditor->DoButton_Editor(&s_CancelButton, "Cancel", 0, &Button, 0, nullptr))

src/game/mapitems.h

@@ -213,6 +213,9 @@ enum
 	ENTITY_OFFSET = 255 - 16 * 4,
 };
 
+static constexpr size_t MAX_MAPIMAGES = 64;
+static constexpr size_t MAX_MAPSOUNDS = 64;
+
 typedef ivec2 CPoint; // 22.10 fixed point
 typedef ivec4 CColor;
 

src/tools/map_convert_07.cpp

@@ -17,13 +17,13 @@ CDataFileReader g_DataReader;
 CDataFileWriter g_DataWriter;
 
 // global new image data (set by ReplaceImageItem)
-int g_aNewDataSize[64];
+int g_aNewDataSize[MAX_MAPIMAGES];
-void *g_apNewData[64];
+void *g_apNewData[MAX_MAPIMAGES];
 
 int g_Index = 0;
 int g_NextDataItemID = -1;
 
-int g_aImageIDs[64];
+int g_aImageIDs[MAX_MAPIMAGES];
 
 int LoadPNG(CImageInfo *pImg, const char *pFilename)
 {
@@ -195,20 +195,25 @@ int main(int argc, const char **argv)
 
 	g_NextDataItemID = g_DataReader.NumData();
 
-	int i = 0;
+	size_t i = 0;
 	for(int Index = 0; Index < g_DataReader.NumItems(); Index++)
 	{
 		int Type;
 		g_DataReader.GetItem(Index, &Type);
 		if(Type == MAPITEMTYPE_IMAGE)
-			g_aImageIDs[i++] = Index;
+		{
+			if(i >= MAX_MAPIMAGES)
+			{
+				dbg_msg("map_convert_07", "map uses more images than the client maximum of %" PRIzu ". filename='%s'", MAX_MAPIMAGES, pSourceFileName);
+				break;
+			}
+			g_aImageIDs[i] = Index;
+			i++;
+		}
 	}
 
 	bool Success = true;
 
-	if(i > 64)
-		dbg_msg("map_convert_07", "%s: Uses more textures than the client maximum of 64.", pSourceFileName);
-
 	// add all items
 	for(int Index = 0; Index < g_DataReader.NumItems(); Index++)
 	{

src/tools/map_optimize.cpp

@@ -115,11 +115,11 @@ int main(int argc, const char **argv)
 		return -1;
 	}
 
-	int aImageFlags[64] = {
+	int aImageFlags[MAX_MAPIMAGES] = {
 		0,
 	};
 
-	bool aaImageTiles[64][256]{
+	bool aaImageTiles[MAX_MAPIMAGES][256]{
 		{
 			false,
 		},
@@ -154,7 +154,7 @@ int main(int argc, const char **argv)
 			if(pLayer->m_Type == LAYERTYPE_TILES)
 			{
 				CMapItemLayerTilemap *pTLayer = (CMapItemLayerTilemap *)pLayer;
-				if(pTLayer->m_Image >= 0 && pTLayer->m_Image < 64 && pTLayer->m_Flags == 0)
+				if(pTLayer->m_Image >= 0 && pTLayer->m_Image < (int)MAX_MAPIMAGES && pTLayer->m_Flags == 0)
 				{
 					aImageFlags[pTLayer->m_Image] |= 1;
 					// check tiles that are used in this image
@@ -180,7 +180,7 @@ int main(int argc, const char **argv)
 			else if(pLayer->m_Type == LAYERTYPE_QUADS)
 			{
 				CMapItemLayerQuads *pQLayer = (CMapItemLayerQuads *)pLayer;
-				if(pQLayer->m_Image >= 0 && pQLayer->m_Image < 64)
+				if(pQLayer->m_Image >= 0 && pQLayer->m_Image < (int)MAX_MAPIMAGES)
 				{
 					aImageFlags[pQLayer->m_Image] |= 2;
 				}