From 6a297effc1b93f505881dc18d1e278b6855729a5 Mon Sep 17 00:00:00 2001
From: lotodore <pokerth@lotharmay.de>
Date: Thu, 18 Oct 2007 23:00:55 +0000
Subject: [PATCH] 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             |  3 ++
 src/core/common/avatarmanager.cpp    | 76 ++++++++++++++++++++++++----
 src/net/common/serverlobbythread.cpp |  2 +-
 3 files changed, 70 insertions(+), 11 deletions(-)

diff --git a/src/core/avatarmanager.h b/src/core/avatarmanager.h
index da9604b2..79db02cc 100644
--- a/src/core/avatarmanager.h
+++ b/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;
 
diff --git a/src/core/common/avatarmanager.cpp b/src/core/common/avatarmanager.cpp
index a923f630..86c31823 100644
--- a/src/core/common/avatarmanager.cpp
+++ b/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)
-				retVal = fileState;
+			if (outFileSize >= MIN_AVATAR_FILE_SIZE && outFileSize <= MAX_AVATAR_FILE_SIZE)
+			{
+				// 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);
-			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);
+			// Check header before storing file.
+			if (IsValidAvatarFileType(avatarFileType, data, size))
 			{
-				boost::mutex::scoped_lock lock(m_cachedAvatarsMutex);
-				m_cachedAvatars.insert(AvatarMap::value_type(md5buf, fileName));
+				path tmpPath(m_cacheDir);
+				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)
 {
diff --git a/src/net/common/serverlobbythread.cpp b/src/net/common/serverlobbythread.cpp
index b8930f34..a59b9493 100644
--- a/src/net/common/serverlobbythread.cpp
+++ b/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);