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
, searching around it, send me to the delete character method in fieldiocpsocket.cpp.
original code of this function:
so start reading it:
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:
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 tt: (and it's will arrive as I actually help many new 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
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 tt: (and it's will arrive as I actually help many new server)