Welcome!

Join our community of MMO enthusiasts and game developers! By registering, you'll gain access to discussions on the latest developments in MMO server files and collaborate with like-minded individuals. Join us today and unlock the potential of MMO server development!

Join Today!

search, find, and remove "in code" security issue (follow it and patch your server)

Skilled Illusionist
Joined
Jul 10, 2008
Messages
371
Reaction score
94
search, find, and remove "in code" security issue (follow it and patch your server)

Hi all I will share here some little piece of code to help you to get out common server attack, I will not speak of massang backdoor command as everyone now know how to remove it from all kind of source.

some day ago some is coming asking me some help for some security trouble, his installation was crappy and his server was open from everywhere, I have give him some advice and recommended him to get back from scratch from his install following some advice (cheap one), like tools only allowed in local area network, db only accessible from localhost and so on.

after sometime to work they got a new dedicated ready to launch their server, but some attack continue, so I have choose to help them personally (this kind of attack from "guys who think they are the best", just kill the game more than he already is), so I looked during 1 day what happen on their server, behind my keyboard for some hour.

first attack (and only one found actually), was a soft delete of player (and admin) gear (race >= 16384).
I took my source code search for 16384 and see it's correspond to a define
Code:
#define RACE_DELETED_CHARACTER		(USHORT)0x4000	// 삭제된 캐릭터, 16384 // 2007-02-21 by cmkwon
, searching around it, send me to the delete character method in fieldiocpsocket.cpp.

original code of this function:
Code:
ProcessResult CFieldIOCPSocket::Process_FC_CHARACTER_DELETE(const char* pPacket, int nLength, int &nBytesUsed)
{
	if (FALSE == IS_VALID_CLIENT_INDEX(m_character.ClientIndex))
	{
		g_pFieldGlobal->WriteSystemLogEX(TRUE, "HACKUSER!! Connect Process_FC_CHARACTER_DELETE Command Using: HackingIP(%15s)\r\n", this->GetPeerIP());
		return RES_RETURN_FALSE;
	}
	char					*pRecvMsg = NULL;
	int						nRecvTypeSize	= 0;
	CFieldIOCPSocket		*pFieldIOCPSocket	= NULL;
	const CHARACTER			*pCharacter			= NULL;
	MSG_FC_CHARACTER_DELETE	*pMsgDel			= NULL;

	// Client로부터 Character를 삭제하는 요청을 받음
	// 응답으로 T_FC_CHARACTER_DELETE_OK 메세지를 보낸다.

	nRecvTypeSize = sizeof(MSG_FC_CHARACTER_DELETE);
	if(nLength - nBytesUsed < nRecvTypeSize)
	{
		// Protocl Error 처리
		// - Client로 부터 받은 Data Size가 Field Type에 따른 Data Size보다 작다
		// Error Code : ERR_PROTOCOL_INVALID_FIELD_DATA
		SendErrorMessage(T_FC_CHARACTER_DELETE, ERR_PROTOCOL_INVALID_FIELD_DATA);
		Close(0x1400C);
		return RES_RETURN_FALSE;
	}
	pRecvMsg = new char[sizeof(MSG_FC_CHARACTER_DELETE)];
	memcpy(pRecvMsg, pPacket+nBytesUsed, nRecvTypeSize);
	nBytesUsed += nRecvTypeSize;

	///////////////////////////////////////////////////////////////////////////////
	// 2006-08-04 by cmkwon, 서버 다운 진행상태
	if(ms_pFieldIOCP->m_bPrepareShutDown)
	{
		SendErrorMessage(T_FC_CHARACTER_DELETE, ERR_DOING_SERVER_SHUTDOWN);
		return RES_BREAK;
	}

	pMsgDel = (MSG_FC_CHARACTER_DELETE*)pRecvMsg;
	if( pMsgDel->AccountUniqueNumber == 0)
	{
		// Protocol Error 처리
		// - 접속한 AccountUniqueNumber와 요청한 AccountUniqueNumber이 다르다
		// Error Code : ERR_PROTOCOL_INVALID_ACCOUNT_UNIQUENUMBER
		SendErrorMessage(T_FC_CHARACTER_DELETE, ERR_PROTOCOL_INVALID_ACCOUNT_UNIQUENUMBER);
		return RES_BREAK;
	}
	
	if ( pMsgDel->CharacterUniqueNumber == 0)
	{
		// Protocol Error 처리
		// - 요청한 CharacterUniqueNumber가 유효하지 않음
		// Error Code : ERR_PROTOCOL_INVALID_CHARACTER_UNIQUENUMBER
		SendErrorMessage(T_FC_CHARACTER_DELETE, ERR_PROTOCOL_INVALID_CHARACTER_UNIQUENUMBER);
		return RES_BREAK;
	}

	///////////////////////////////////////////////////////////////////////////////
	// 2007-09-13 by cmkwon, 베트남 2차패스워드 구현 - 캐릭터 삭제시 2차패스워드 체크
	if(g_pFieldGlobal->GetUseSecondaryPasswordSystemFlag()
		&& this->IsSettingSecondaryPassword())
	{
		if(FALSE == this->CompareSecondaryPassword(pMsgDel->CurrentSecPassword))
		{
			SendErrorMessage(T_FC_CHARACTER_DELETE, ERR_SECPASS_PASSWORD_NOT_MATCHED);
			return RES_BREAK;
		}
	}
	
	//////////////////////////////////////////////////////////////////////////
	// 2007-11-09 by dhjin, 전진기지 관련하여 처리
	if(IS_VALID_UNIQUE_NUMBER(this->m_character.GuildUniqueNumber)				// 2007-12-18 by dhjin, 버그 수정
		&& ms_pFieldIOCP->m_OutPostManager.CheckALLOutPostWaring()
		&& ms_pFieldIOCP->m_OutPostManager.CheckOutPostPossessByGuildUID(this->m_character.GuildUniqueNumber))
	{
		SendErrorMessage(T_FC_CHARACTER_DELETE, ERR_WARING_OUTPOST_TO_OWNMAP);
		return RES_BREAK;
	}

	if (this->client)
		
	// 처리
	ms_pFieldIOCP->m_pAtumDBManager->MakeAndEnqueueQuery(QT_DeleteCharacter, this, pMsgDel->AccountUniqueNumber, (MSG_FC_CHARACTER_DELETE*)pRecvMsg);
	pRecvMsg = NULL;
	return RES_RETURN_TRUE;
}

so start reading it:
Code:
if (FALSE == IS_VALID_CLIENT_INDEX(m_character.ClientIndex))
	{
		g_pFieldGlobal->WriteSystemLogEX(TRUE, "HACKUSER!! Connect Process_FC_CHARACTER_DELETE Command Using: HackingIP(%15s)\r\n", this->GetPeerIP());
		return RES_RETURN_FALSE;
	}

this piece of code let us think we are protected but not really is just check the actual instance of the socket have a character (not necessary full filled) with a valid client index (I let you take a look to the IS_VALID_CLIENT_INDEX macro as this one is not hard to understand.

so what happen if someone get a valid client index and send in our field server a delete packet with account id and char id of someone else??

it will simply work fine (you just have to fetch some packet where you are connected to fetch player account/character id ;)), so how ensure it's will not arrive again?

juste by adding a small check on account id like it:
Code:
...
pMsgDel = (MSG_FC_CHARACTER_DELETE*)pRecvMsg;
	if( pMsgDel->AccountUniqueNumber == 0 || pMsgDel->AccountUniqueNumber != m_character.AccountUniqueNumber)
	{
		// Protocol Error 처리
		// - 접속한 AccountUniqueNumber와 요청한 AccountUniqueNumber이 다르다
		// Error Code : ERR_PROTOCOL_INVALID_ACCOUNT_UNIQUENUMBER
		SendErrorMessage(T_FC_CHARACTER_DELETE, ERR_PROTOCOL_INVALID_ACCOUNT_UNIQUENUMBER);
		return RES_BREAK;
	}
...

just checking the connected account is same as the one we focus to delete a character.

I will let this thread up to date when I found new code security fail.

special thanks to the attacker of concerned server, hope to cross you again :eek:tt: (and it's will arrive as I actually help many new server)
 
Joined
Apr 12, 2013
Messages
896
Reaction score
479
Re: search, find, and remove "in code" security issue (follow it and patch your serve

Looks like this kind of attack here:

Code:
PacketHeader.MSG_FC_CHARACTER_DELETE packet = new PacketHeader.MSG_FC_CHARACTER_DELETE();
packet.AccountUniqueNumber = 1;
for(uint i = 0; i < 10000; i++)
{
     packet.CharacterUniqueNumber = i;              
     g_pSocket.SendPacket(PacketHeader.T_FC_CHARACTER_DELETE, packet);
}

So i may kinda know who did the attack :stupido2:
 
Skilled Illusionist
Joined
Jul 10, 2008
Messages
371
Reaction score
94
Re: search, find, and remove "in code" security issue (follow it and patch your serve

if you have other in reserve send it it's like a game for me ^^'
 
Joined
Apr 12, 2013
Messages
896
Reaction score
479
Re: search, find, and remove "in code" security issue (follow it and patch your serve

I ain't using this persons buggy tool, more crashes on tool side than on the server side lol
 
Joined
Apr 12, 2013
Messages
896
Reaction score
479
Re: search, find, and remove "in code" security issue (follow it and patch your serve

I also will throw a little fix for an exploit used to kick all player from the server:

The attacker send several T_FI_CONNECT_NOTIFY_GAMEEND messages with a few character numbers, to bruteforce the disconnects like this:
Code:
for(int i = 1; i < 10000; i++)
{
    Packet.characterUID = (uint)i;
    g_pSocket.SendBypassPacket(PacketHeader.T_PC_CONNECT_ALIVE, PacketHeader.T_FI_CONNECT_NOTIFY_GAMEEND, Packet);
}

But the question is, why does this work? We have a Field -> IM Packet only, visible because it's named T_FI. So basically the IM server isn't checking if the send packet comes from IM Server or not, so basically everyone could connect and send this packet.

The fix here now is really simple, just add a check if the current socket is the FieldServer Socket or not.

Simply search for the handler of the packet:
Code:
ProcessResult CIMIOCPSocket::Process_FI_CONNECT_NOTIFY_GAMEEND(const char* pPacket, int nLength, int &nBytesUsed)
in IMIOCPSocket.cpp

and there you can see, there is no such check.
Luckily every server has a few variables or methods to get the IM/Field/NPC/Pre Sockets,
in the IM server it's a simple variable
Code:
ms_pIMIOCP->m_pFieldServerSocket

so add the red colored lines into this method:


Code:
nRecvTypeSize = sizeof(MSG_FI_CONNECT_NOTIFY_GAMEEND);
if(nLength - nBytesUsed < nRecvTypeSize)
{
    // Protocl Error ó¸®
    // - Client·Î ºÎÅÍ ¹ÞÀº Data Size°¡ Field Type¿¡ µû¸¥ Data Sizeº¸´Ù ÀÛ´Ù    
    SendErrorMessage(T_FI_CONNECT_NOTIFY_GAMEEND, ERR_PROTOCOL_INVALID_FIELD_DATA);    
    return RES_RETURN_FALSE;
}
pMsgNotifyGameEnd = (MSG_FI_CONNECT_NOTIFY_GAMEEND*)(pPacket+nBytesUsed);
nBytesUsed += nRecvTypeSize;

[COLOR=#ff0000]if (this != ms_pIMIOCP->m_pFieldServerSocket)
    return RES_RETURN_FALSE;

[/COLOR]CIMIOCPSocket *pIMSocket = ms_pIMIOCP->GetIMIOCPSocketByCharacterUID(pMsgNotifyGameEnd->CharacterUniqueNumber);

this checks if the current socket is the FieldServer, and when not should disconnect the socket immediately.

There may be several other vulnerabilities in the servers, but you can simply try to use this kind of fix to fix them.
 
Skilled Illusionist
Joined
Jul 10, 2008
Messages
371
Reaction score
94
Re: search, find, and remove "in code" security issue (follow it and patch your serve

Nice one thanks for the contribution to this thread (I think lot of packet handler must have this kind of fail, I will check them one by one)
 
Skilled Illusionist
Joined
Jul 10, 2008
Messages
371
Reaction score
94
Re: search, find, and remove "in code" security issue (follow it and patch your serve

I will search them and post them, your post is very helpful and give me a new way to find them quickly(just check process_XXXXXX method)
I'm happy to see someone participating to this thread.
 
Experienced Elementalist
Joined
Oct 9, 2012
Messages
226
Reaction score
76
Re: search, find, and remove "in code" security issue (follow it and patch your serve

Almost or every bin<>bin packet need to be protected. Also theres a few vulnerable client packets. Guild dismember for example.
 
Skilled Illusionist
Joined
Jul 10, 2008
Messages
371
Reaction score
94
Re: search, find, and remove "in code" security issue (follow it and patch your serve

yeah, that is the case for bin to bin request, thanks for guild dismember information, I will look at it too.
 
Skilled Illusionist
Joined
Jul 10, 2008
Messages
371
Reaction score
94
Re: search, find, and remove "in code" security issue (follow it and patch your serve

New patch:
account block packet fail:

in preserver (preIOCPSocket.cpp) found the handler for T_FP_ADMIN_BLOCKACCOUNT packet type.

this part of my source code has been rework (by pankj) and so you will not find exactly this snipet of code.

Code:
template<> ProcessResult CPreIOCPSocket::HandlerT1<T_FP_ADMIN_BLOCKACCOUNT>::handler(CPreIOCPSocket* socket, const char* pPacket, int nLength, int &nBytesUsed)
{
	[COLOR="#FF0000"]//check send by field server
	CServerGroup *pServerGroup = ms_pPreIOCP->GetServerGroup(socket->m_szConnectedServerGroupName);
	if (pServerGroup == NULL)
	{
		DBGOUT("  Error: No Such Server Group(%s)!", socket->m_szConnectedServerGroupName);
		return RES_PACKET_ERROR;
	}
	CPreIOCPSocket *pPFSocket = (CPreIOCPSocket*)pServerGroup->m_FieldServerInfo.pSocket;
	if (pPFSocket != socket) return RES_PACKET_ERROR;[/COLOR]
	
	auto msg = GetMessageData(pPacket, nLength, nBytesUsed);
	if (!msg) return RES_PACKET_ERROR;

	///////////////////////////////////////////////////////////////////////////////
	// 2008-01-31 by cmkwon, °èÁ¤ ºí·°/ÇØÁ¦ ¸í·É¾î·Î °¡´ÉÇÑ ½Ã½ºÅÛ ±¸Çö - 
	server::log(TRUE, "[Notify] Account Block: AdminAccountName(%20s), BlockedUserAccName(%20s) Period(%s ~ %s)\r\n"
		, msg->blockAccInfo.szBlockAdminAccountName, msg->blockAccInfo.szBlockedAccountName
		, msg->blockAccInfo.atimeStartTime.GetDateTimeString().GetBuffer()
		, msg->blockAccInfo.atimeEndTime.GetDateTimeString().GetBuffer());

	ms_pPreIOCP->BlockAccount(&msg->blockAccInfo, socket, TRUE);
	return RES_BREAK;
}

the red part is what i have added to check the packet come from the field server of my own group name (setup in global.cfg)

in the code released on this forum the function where you must add this code must be called: process_FP_ADMIN_BLOCKACCOUNT or somthing like that.
 
Back
Top