Improve map image validation

- Add check for map image data being loaded from map to avoid null pointer access.
- Add check for size of map image data to avoid out-of-bounds reads.
- Add check for map layer item not being found to avoid null pointer access.
- Fix inconsistent handling of layers with invalid images. In some cases only `-1` was considered non-textured whereas all invalid values should be.
- Ensure correct texture is always used for game layers instead of considering the image property of game layers which cannot be set via the map editor.
- Skip loading images which are not used by any quads or tiles layers.
- Replace member variable `m_aTextureUsedByTileOrQuadLayerFlag` with local variable.
- Mark pointers as `const` when possible.
- Avoid C-style casts.

src/game/client/components/mapimages.cpp

@@ -37,8 +37,6 @@ CMapImages::CMapImages(int TextureSize)
 	mem_zero(m_aEntitiesIsLoaded, sizeof(m_aEntitiesIsLoaded));
 	m_SpeedupArrowIsLoaded = false;
 
-	mem_zero(m_aTextureUsedByTileOrQuadLayerFlag, sizeof(m_aTextureUsedByTileOrQuadLayerFlag));
-
 	str_copy(m_aEntitiesPath, "editor/entities_clear");
 
 	static_assert(std::size(gs_apModEntitiesNames) == MAP_IMAGE_MOD_TYPE_COUNT, "Mod name string count is not equal to mod type count");
@@ -61,41 +59,44 @@ void CMapImages::OnMapLoadImpl(class CLayers *pLayers, IMap *pMap)
 	// unload all textures
 	for(int i = 0; i < m_Count; i++)
 	{
-		Graphics()->UnloadTexture(&(m_aTextures[i]));
+		Graphics()->UnloadTexture(&m_aTextures[i]);
-		m_aTextureUsedByTileOrQuadLayerFlag[i] = 0;
 	}
-	m_Count = 0;
 
 	int Start;
 	pMap->GetType(MAPITEMTYPE_IMAGE, &Start, &m_Count);
-
 	m_Count = clamp<int>(m_Count, 0, MAX_MAPIMAGES);
 
-	for(int g = 0; g < pLayers->NumGroups(); g++)
+	unsigned char aTextureUsedByTileOrQuadLayerFlag[MAX_MAPIMAGES] = {0}; // 0: nothing, 1(as flag): tile layer, 2(as flag): quad layer
+	for(int GroupIndex = 0; GroupIndex < pLayers->NumGroups(); GroupIndex++)
 	{
-		CMapItemGroup *pGroup = pLayers->GetGroup(g);
+		const CMapItemGroup *pGroup = pLayers->GetGroup(GroupIndex);
 		if(!pGroup)
 		{
 			continue;
 		}
 
-		for(int l = 0; l < pGroup->m_NumLayers; l++)
+		for(int LayerIndex = 0; LayerIndex < pGroup->m_NumLayers; LayerIndex++)
 		{
-			CMapItemLayer *pLayer = pLayers->GetLayer(pGroup->m_StartLayer + l);
+			const CMapItemLayer *pLayer = pLayers->GetLayer(pGroup->m_StartLayer + LayerIndex);
+			if(!pLayer)
+			{
+				continue;
+			}
+
 			if(pLayer->m_Type == LAYERTYPE_TILES)
 			{
-				CMapItemLayerTilemap *pTLayer = (CMapItemLayerTilemap *)pLayer;
+				const CMapItemLayerTilemap *pLayerTilemap = reinterpret_cast<const CMapItemLayerTilemap *>(pLayer);
-				if(pTLayer->m_Image >= 0 && pTLayer->m_Image < m_Count)
+				if(pLayerTilemap->m_Image >= 0 && pLayerTilemap->m_Image < m_Count)
 				{
-					m_aTextureUsedByTileOrQuadLayerFlag[pTLayer->m_Image] |= 1;
+					aTextureUsedByTileOrQuadLayerFlag[pLayerTilemap->m_Image] |= 1;
 				}
 			}
 			else if(pLayer->m_Type == LAYERTYPE_QUADS)
 			{
-				CMapItemLayerQuads *pQLayer = (CMapItemLayerQuads *)pLayer;
+				const CMapItemLayerQuads *pLayerQuads = reinterpret_cast<const CMapItemLayerQuads *>(pLayer);
-				if(pQLayer->m_Image >= 0 && pQLayer->m_Image < m_Count)
+				if(pLayerQuads->m_Image >= 0 && pLayerQuads->m_Image < m_Count)
 				{
-					m_aTextureUsedByTileOrQuadLayerFlag[pQLayer->m_Image] |= 2;
+					aTextureUsedByTileOrQuadLayerFlag[pLayerQuads->m_Image] |= 2;
 				}
 			}
 		}
@@ -107,8 +108,14 @@ void CMapImages::OnMapLoadImpl(class CLayers *pLayers, IMap *pMap)
 	bool ShowWarning = false;
 	for(int i = 0; i < m_Count; i++)
 	{
-		const int LoadFlag = (((m_aTextureUsedByTileOrQuadLayerFlag[i] & 1) != 0) ? TextureLoadFlag : 0) | (((m_aTextureUsedByTileOrQuadLayerFlag[i] & 2) != 0) ? 0 : (Graphics()->HasTextureArraysSupport() ? IGraphics::TEXLOAD_NO_2D_TEXTURE : 0));
+		if(aTextureUsedByTileOrQuadLayerFlag[i] == 0)
-		const CMapItemImage_v2 *pImg = (CMapItemImage_v2 *)pMap->GetItem(Start + i);
+		{
+			// skip loading unused images
+			continue;
+		}
+
+		const int LoadFlag = (((aTextureUsedByTileOrQuadLayerFlag[i] & 1) != 0) ? TextureLoadFlag : 0) | (((aTextureUsedByTileOrQuadLayerFlag[i] & 2) != 0) ? 0 : (Graphics()->HasTextureArraysSupport() ? IGraphics::TEXLOAD_NO_2D_TEXTURE : 0));
+		const CMapItemImage_v2 *pImg = static_cast<const CMapItemImage_v2 *>(pMap->GetItem(Start + i));
 
 		const char *pName = pMap->GetDataString(pImg->m_ImageName);
 		if(pName == nullptr || pName[0] == '\0')
@@ -151,10 +158,20 @@ void CMapImages::OnMapLoadImpl(class CLayers *pLayers, IMap *pMap)
 			ImageInfo.m_Height = pImg->m_Height;
 			ImageInfo.m_Format = CImageInfo::FORMAT_RGBA;
 			ImageInfo.m_pData = static_cast<uint8_t *>(pMap->GetData(pImg->m_ImageData));
-			char aTexName[IO_MAX_PATH_LENGTH];
+			if(ImageInfo.m_pData && (size_t)pMap->GetDataSize(pImg->m_ImageData) >= ImageInfo.DataSize())
-			str_format(aTexName, sizeof(aTexName), "embedded: %s", pName);
+			{
-			m_aTextures[i] = Graphics()->LoadTextureRaw(ImageInfo, LoadFlag, aTexName);
+				char aTexName[IO_MAX_PATH_LENGTH];
-			pMap->UnloadData(pImg->m_ImageData);
+				str_format(aTexName, sizeof(aTexName), "embedded: %s", pName);
+				m_aTextures[i] = Graphics()->LoadTextureRaw(ImageInfo, LoadFlag, aTexName);
+				pMap->UnloadData(pImg->m_ImageData);
+			}
+			else
+			{
+				pMap->UnloadData(pImg->m_ImageData);
+				log_error("mapimages", "Failed to load map image %d: failed to load data.", i);
+				ShowWarning = true;
+				continue;
+			}
 		}
 		pMap->UnloadData(pImg->m_ImageName);
 		ShowWarning = ShowWarning || m_aTextures[i].IsNullTexture();

src/game/client/components/mapimages.h

@@ -37,7 +37,6 @@ class CMapImages : public CComponent
 	friend class CMenuBackground;
 
 	IGraphics::CTextureHandle m_aTextures[MAX_MAPIMAGES];
-	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

@@ -386,15 +386,8 @@ void CMapLayers::OnMapLoad()
 
 			if(pLayer->m_Type == LAYERTYPE_TILES && Graphics()->IsTileBufferingEnabled())
 			{
-				bool DoTextureCoords = false;
 				CMapItemLayerTilemap *pTMap = (CMapItemLayerTilemap *)pLayer;
-				if(pTMap->m_Image == -1)
+				const bool DoTextureCoords = IsEntityLayer || (pTMap->m_Image >= 0 && pTMap->m_Image < m_pImages->Num());
-				{
-					if(IsEntityLayer)
-						DoTextureCoords = true;
-				}
-				else
-					DoTextureCoords = true;
 
 				int DataIndex = 0;
 				unsigned int TileSize = 0;
@@ -740,7 +733,7 @@ void CMapLayers::OnMapLoad()
 				m_vpQuadLayerVisuals.push_back(new SQuadLayerVisuals());
 				SQuadLayerVisuals *pQLayerVisuals = m_vpQuadLayerVisuals.back();
 
-				bool Textured = (pQLayer->m_Image != -1);
+				const bool Textured = pQLayer->m_Image >= 0 && pQLayer->m_Image < m_pImages->Num();
 
 				vtmpQuads.clear();
 				vtmpQuadsTextured.clear();
@@ -1543,15 +1536,18 @@ void CMapLayers::OnRender()
 				if(pLayer->m_Type == LAYERTYPE_TILES)
 				{
 					CMapItemLayerTilemap *pTMap = (CMapItemLayerTilemap *)pLayer;
-					if(pTMap->m_Image < 0 || pTMap->m_Image >= m_pImages->Num())
+					if(IsGameLayer)
 					{
-						if(!IsGameLayer)
+						Graphics()->TextureSet(m_pImages->GetEntities(MAP_IMAGE_ENTITY_LAYER_TYPE_ALL_EXCEPT_SWITCH));
-							Graphics()->TextureClear();
+					}
-						else
+					else if(pTMap->m_Image >= 0 && pTMap->m_Image < m_pImages->Num())
-							Graphics()->TextureSet(m_pImages->GetEntities(MAP_IMAGE_ENTITY_LAYER_TYPE_ALL_EXCEPT_SWITCH));
+					{
+						Graphics()->TextureSet(m_pImages->Get(pTMap->m_Image));
 					}
 					else
-						Graphics()->TextureSet(m_pImages->Get(pTMap->m_Image));
+					{
+						Graphics()->TextureClear();
+					}
 
 					CTile *pTiles = (CTile *)m_pLayers->Map()->GetData(pTMap->m_Data);
 					unsigned int Size = m_pLayers->Map()->GetDataSize(pTMap->m_Data);
@@ -1608,10 +1604,14 @@ void CMapLayers::OnRender()
 				else if(pLayer->m_Type == LAYERTYPE_QUADS)
 				{
 					CMapItemLayerQuads *pQLayer = (CMapItemLayerQuads *)pLayer;
-					if(pQLayer->m_Image < 0 || pQLayer->m_Image >= m_pImages->Num())
+					if(pQLayer->m_Image >= 0 && pQLayer->m_Image < m_pImages->Num())
-						Graphics()->TextureClear();
+					{
-					else
 						Graphics()->TextureSet(m_pImages->Get(pQLayer->m_Image));
+					}
+					else
+					{
+						Graphics()->TextureClear();
+					}
 
 					CQuad *pQuads = (CQuad *)m_pLayers->Map()->GetDataSwapped(pQLayer->m_Data);
 					if(m_Type == TYPE_BACKGROUND_FORCE || m_Type == TYPE_FULL_DESIGN)