Try to prevent (as much as possible) transfering and, more important, storing, any other files than avatars as png, gif and jpg. The headers of the files are checked whether they are valid for the corresponding format. Files without gif/png/jpg header are never stored on the server, and thus cannot be requested by any client.

src/core/avatarmanager.h

@@ -29,6 +29,7 @@
 #include <boost/thread.hpp>
 #include <boost/shared_ptr.hpp>
 
+#define MIN_AVATAR_FILE_SIZE	32
 #define MAX_AVATAR_FILE_SIZE	30720
 
 struct AvatarFileState;
@@ -52,6 +53,8 @@ public:
 	bool HasAvatar(const MD5Buf &md5buf) const;
 	bool StoreAvatarInCache(const MD5Buf &md5buf, AvatarFileType avatarFileType, const unsigned char *data, unsigned size);
 
+	static bool IsValidAvatarFileType(AvatarFileType avatarFileType, const unsigned char *fileHeader, unsigned fileHeaderSize);
+
 protected:
 	typedef std::map<MD5Buf, std::string> AvatarMap;
 

src/core/common/avatarmanager.cpp

@@ -27,6 +27,15 @@
 #include <fstream>
 #include <cstring>
 
+#define PNG_HEADER "\x89\x50\x4e\x47\x0d\x0a\x1a\x0a"
+#define PNG_HEADER_SIZE (sizeof(PNG_HEADER) - 1)
+#define JPG_HEADER "\xff\xd8"
+#define JPG_HEADER_SIZE (sizeof(JPG_HEADER) - 1)
+#define GIF_HEADER_1 "GIF87a"
+#define GIF_HEADER_2 "GIF89a"
+#define GIF_HEADER_SIZE (sizeof(GIF_HEADER_1) - 1)
+#define MAX_HEADER_SIZE PNG_HEADER_SIZE
+
 // Not using boost::algorithm here because of STL issues.
 #ifdef _MSC_VER
 #define STRCASECMP _stricmp
@@ -103,8 +112,16 @@ AvatarManager::OpenAvatarFileForChunkRead(const std::string &fileName, unsigned
 			fileState->inputStream.seekg(0, ios_base::beg);
 			std::streamoff posDiff(endPos - startPos);
 			outFileSize = (unsigned)posDiff;
-			if (outFileSize <= MAX_AVATAR_FILE_SIZE)
+			if (outFileSize >= MIN_AVATAR_FILE_SIZE && outFileSize <= MAX_AVATAR_FILE_SIZE)
-				retVal = fileState;
+			{
+				// Validate type of file by verifying image header.
+				unsigned char fileHeader[MAX_HEADER_SIZE];
+				fileState->inputStream.read((char *)fileHeader, sizeof(fileHeader));
+				fileState->inputStream.seekg(0, ios_base::beg);
+
+				if (IsValidAvatarFileType(outFileType, fileHeader, sizeof(fileHeader)))
+					retVal = fileState;
+			}
 		}
 	} catch (...)
 	{
@@ -288,16 +305,20 @@ AvatarManager::StoreAvatarInCache(const MD5Buf &md5buf, AvatarFileType avatarFil
 		}
 		if (!ext.empty())
 		{
-			path tmpPath(m_cacheDir);
+			// Check header before storing file.
-			tmpPath /= (md5buf.ToString() + ext);
+			if (IsValidAvatarFileType(avatarFileType, data, size))
-			string fileName(tmpPath.file_string());
-			ofstream o(fileName.c_str(), ios_base::out | ios_base::binary);
-			o.write((const char *)data, size);
 			{
-				boost::mutex::scoped_lock lock(m_cachedAvatarsMutex);
+				path tmpPath(m_cacheDir);
-				m_cachedAvatars.insert(AvatarMap::value_type(md5buf, fileName));
+				tmpPath /= (md5buf.ToString() + ext);
+				string fileName(tmpPath.file_string());
+				ofstream o(fileName.c_str(), ios_base::out | ios_base::binary);
+				o.write((const char *)data, size);
+				{
+					boost::mutex::scoped_lock lock(m_cachedAvatarsMutex);
+					m_cachedAvatars.insert(AvatarMap::value_type(md5buf, fileName));
+				}
+				retVal = true;
 			}
-			retVal = true;
 		}
 	} catch (...)
 	{
@@ -305,6 +326,41 @@ AvatarManager::StoreAvatarInCache(const MD5Buf &md5buf, AvatarFileType avatarFil
 	return retVal;
 }
 
+bool
+AvatarManager::IsValidAvatarFileType(AvatarFileType avatarFileType, const unsigned char *fileHeader, unsigned fileHeaderSize)
+{
+	bool validType = false;
+
+	switch (avatarFileType)
+	{
+		case AVATAR_FILE_TYPE_PNG:
+			if (fileHeaderSize >= PNG_HEADER_SIZE
+				&& memcmp(fileHeader, PNG_HEADER, PNG_HEADER_SIZE) == 0)
+			{
+				validType = true;
+			}
+			break;
+		case AVATAR_FILE_TYPE_JPG:
+			if (fileHeaderSize >= JPG_HEADER_SIZE
+				&& memcmp(fileHeader, JPG_HEADER, JPG_HEADER_SIZE) == 0)
+			{
+				validType = true;
+			}
+			break;
+		case AVATAR_FILE_TYPE_GIF:
+			if (fileHeaderSize >= GIF_HEADER_SIZE
+				&& (memcmp(fileHeader, GIF_HEADER_1, GIF_HEADER_SIZE) == 0
+					|| memcmp(fileHeader, GIF_HEADER_2, GIF_HEADER_SIZE) == 0))
+			{
+				validType = true;
+			}
+			break;
+		case AVATAR_FILE_TYPE_UNKNOWN:
+			break;
+	}
+	return validType;
+}
+
 bool
 AvatarManager::InternalReadDirectory(const std::string &dir, AvatarMap &avatars)
 {

src/net/common/serverlobbythread.cpp

@@ -417,7 +417,7 @@ ServerLobbyThread::HandleNetPacketAvatarHeader(SessionWrapper session, const Net
 		NetPacketAvatarHeader::Data headerData;
 		tmpPacket.GetData(headerData);
 
-		if (headerData.avatarFileSize && headerData.avatarFileSize <= MAX_AVATAR_FILE_SIZE)
+		if (headerData.avatarFileSize >= MIN_AVATAR_FILE_SIZE && headerData.avatarFileSize <= MAX_AVATAR_FILE_SIZE)
 		{
 			boost::shared_ptr<AvatarData> tmpAvatarData(new AvatarData);
 			tmpAvatarData->fileData.reserve(headerData.avatarFileSize);