[Release] FlyFF PC v21.2 Official Source Code

[miniman06]

BOOL CanAdd( DWORD dwGold, int nPlus )
{
	if( nPlus <= 0 )		// ´õÇÏ·Á´Â °ªÀÌ 0ÀÌÇÏÀÌ¸é ³Í¼¾½º 
	{
		return FALSE;
	}

	__int64 n64Gold = (__int64)dwGold;

	if( n64Gold < 0 || ( n64Gold + (__int64)nPlus ) < 0 || ( n64Gold + (__int64)nPlus ) > INT_MAX )
	{
		//		FLERROR_LOG( PROGRAM_NAME, _T( "CanAdd Invalid. HaveGold:[%d], AddGold:[%d]" ), dwGold, nPlus );
		return FALSE;
	}

	return TRUE;
}

you can tell that they are scared LOL

[ZeroTwo02]

Originally Posted by miniman06

BOOL CanAdd( DWORD dwGold, int nPlus )
{
	if( nPlus <= 0 )		// ´õÇÏ·Á´Â °ªÀÌ 0ÀÌÇÏÀÌ¸é ³Í¼¾½º 
	{
		return FALSE;
	}

	__int64 n64Gold = (__int64)dwGold;

	if( n64Gold < 0 || ( n64Gold + (__int64)nPlus ) < 0 || ( n64Gold + (__int64)nPlus ) > INT_MAX )
	{
		//		FLERROR_LOG( PROGRAM_NAME, _T( "CanAdd Invalid. HaveGold:[%d], AddGold:[%d]" ), dwGold, nPlus );
		return FALSE;
	}

	return TRUE;
}

you can tell that they are scared LOL

Or we can tell you don't understand the point of that.

[miniman06]

Originally Posted by ZeroTwo02

Or we can tell you don't understand the point of that.

Isn't it just a fancy anti int overflow ? I don't this the 64 bit conversion was necessary but whatever floats their boat

[jooodzszsz]

if( nPlus < 0 || (__int64)dwGold + (__int64)nPlus > INT_MAX)
return false

would be enuff sir

[miniman06]

Originally Posted by jooodzszsz

would be enuff sir

Yeah that, but since the server is compiled under 32bit the performance would be worse than just straight comparison of both 32bit values separately.Just neat picking here lol

[Keyzen83]

Ich habe XTrap entfernt, wie Sie sagten, aber es ändert viel an der Datei, da ich die FLLib-Bibliothek nicht sehe oder finde

[jooodzszsz]

Originally Posted by miniman06

Yeah that, but since the server is compiled under 32bit the performance would be worse than just straight comparison of both 32bit values separately.Just neat picking here lol

if( nPlus =< 0 || dwGold => INT_MAX || (nPlus + dwGold) > INT_MAX)
return FALSE;
return TRUE;

or

if(nPlus > 0 && dwGold < INT_MAX){
return (dwGold + nPlus) < INT_MAX;
}
return FALSE;

now its 1 nano second faster per year

[ZeroTwo02]

Btw, if you want to see some "work in progress".
Look into "WndInventoryWear" and this :

[Mike Oxmaul]

Originally Posted by ZeroTwo02

Btw, if you want to see some "work in progress".
Look into "WndInventoryWear" and this :

the taskbar is in the client since years. i dont think its in progress. just concept.

[cookie69]

At least 4 exploits are still not fixed in this version, you can corrupt the stack with bufferoverflow, set a value outside the array limit and make huge memory allocation.

- BOOL CProject::IsInvalidName( LPCSTR szName )
- void CItemUpgrade::RemovePetVisItem( FLWSUser* pUser, int nPosition, BOOL bExpired )
- struct __MINIGAME_EXT_PACKET : public __MINIGAME_PACKET
- int CMiniGame::Bet_FiveSystem( FLWSUser* pUser, int nBetNum, int nBetPenya )

[Mike Oxmaul]

Originally Posted by cookie69

At least 4 exploits are still not fixed in this version, you can corrupt the stack with bufferoverflow, set a value outside the array limit and make huge memory allocation.

- BOOL CProject::IsInvalidName( LPCSTR szName )
- void CItemUpgrade::RemovePetVisItem( FLWSUser* pUser, int nPosition, BOOL bExpired )
- struct __MINIGAME_EXT_PACKET : public __MINIGAME_PACKET
- int CMiniGame::Bet_FiveSystem( FLWSUser* pUser, int nBetNum, int nBetPenya )

there are more in dpsrvr

[Keyzen83]

_Common\ProjectCmn.cpp

if( strlen( szName ) >= 64 )
return TRUE;

WORLDSERVER\ItemUpgrade.cpp

if( nPosition < 0 || nPosition >= pItemElemPet->GetPiercingSize() )
return;

_Common\MiniGameBase.h

if(nSize > 3)
{
nSize = 3;
}

_Common\MiniGame.cpp

int CMiniGame::Bet_FiveSystem( FLWSUserr* pUser, int nBetNum, int nBetPenya )
{
if( nBetNum < 0 || 5 < nBetNum )
return FIVESYSTEM_FAILED;

if( pUser->m_nBetFiveSystem[nBetNum] == 0 && nBetPenya == 0 )
return TRUE;
// 입찰금은 0보다 작을 수 없다.
if(nBetPenya < 0)
return FIVESYSTEM_FAILED;

// 최소 입찰금과 최대 입찰금의 범위를 벗어났을 경우 ( Client 에서 이미 검사.. )
if( ( nBetPenya < m_nBetMinPenya || nBetPenya > m_nBetMaxPenya ) && nBetPenya != 0 )
return FIVESYSTEM_NOTENOUGH;

// 입찰 또는 재입찰 금액이 부족한지 검사
DWORD nGold = pUser->GetGold() + pUser->m_nBetFiveSystem[nBetNum];
if( (int)( nGold ) < nBetPenya )
return FIVESYSTEM_NOTENOUGH;

LogItemInfo aLogItem;
aLogItem.RecvName = "FIVESYSTEM_USE";
BOOL bAddGold = TRUE;
if( 0 < pUser->m_nBetFiveSystem[nBetNum] - nBetPenya ) // 기존에 걸은것보다 적게 걸아서 보상금액
{
int nTotal = pUser->GetGold() + pUser->m_nBetFiveSystem[nBetNum] - nBetPenya;
if( nTotal < 0 ) // overflow?
{
CItemElem ItemElem;
g_dpDBClient.SendQueryPostMail( pUser->m_idPlayer, 0, ItemElem, pUser->m_nBetFiveSystem[nBetNum] - nBetPenya, (char*)GETTEXT( TID_GAME_SELLING_COST_0 ), (char*)GETTEXT( TID_MMI_FIVESYSTEM ) );
pUser->AddDefinedText( TID_GAME_MINIGAME_PENYA_POST, "" );
aLogItem.RecvName = "FIVESYSTEM_USE_POST";
bAddGold = FALSE;
}
}

if( bAddGold )
pUser->AddGold( pUser->m_nBetFiveSystem[nBetNum] - nBetPenya );

aLogItem.Action = "+";
aLogItem.SendName = pUser->GetName();
aLogItem.WorldId = pUser->GetWorld()->GetID();
aLogItem.Gold = pUser->GetGold() - ( pUser->m_nBetFiveSystem[nBetNum] - nBetPenya );
aLogItem.Gold2 = pUser->GetGold();
//aLogItem.ItemName = "SEED";
_stprintf( aLogItem.szItemName, "%d", II_GOLD_SEED1 );
aLogItem.itemNumber = pUser->m_nBetFiveSystem[nBetNum] - nBetPenya;
g_DPSrvr.OnLogItem( aLogItem );

pUser->m_nBetFiveSystem[nBetNum] = nBetPenya;
pUser->AddFiveSystemResult( FIVESYSTEM_BETOK, nBetPenya, nBetNum );

return TRUE;
}

here is some fix

[jooodzszsz]

Originally Posted by cookie69

At least 4 exploits are still not fixed in this version, you can corrupt the stack with bufferoverflow, set a value outside the array limit and make huge memory allocation.

- BOOL CProject::IsInvalidName( LPCSTR szName )
- void CItemUpgrade::RemovePetVisItem( FLWSUser* pUser, int nPosition, BOOL bExpired )
- struct __MINIGAME_EXT_PACKET : public __MINIGAME_PACKET
- int CMiniGame::Bet_FiveSystem( FLWSUser* pUser, int nBetNum, int nBetPenya )

- void CItemUpgrade::RemovePetVisItem( FLWSUser* pUser, int nPosition, BOOL bExpired )

How you will crash here with nPosition? its impossible? oO

It will be casted to size_t (unsigned) in GetGeneralPiercingItemID() and checked later with "m_vecGeneralPiercing.size() > Nth"

Originally Posted by Jupsi332

there are more in dpsrvr

"There are more bugs in SOURCE but i dont tell them hihihihihi"

[cookie69]

Originally Posted by jooodzszsz

- void CItemUpgrade::RemovePetVisItem( FLWSUser* pUser, int nPosition, BOOL bExpired )

How you will crash here with nPosition? its impossible? oO

It will be casted to size_t (unsigned) in GetGeneralPiercingItemID() and checked later with "m_vecGeneralPiercing.size() > Nth"

"There are more bugs in SOURCE but i dont tell them hihihihihi"

It is not a cast, if you pass a negative Nth to FLItemElem::GetGeneralPiercingItemIDthen then Nth will stay negative.
And later in m_kPiercingOption.GetGeneralPiercingItemID the check will be successful as m_vecGeneralPiercing.size() is always bigger than a negative value.
And later you do "return m_vecGeneralPiercing.at( Nth );" which will search a value at a negative index.

This will not at 100% crash the server but can corrupt the stack and maybe crash it later or write random data into its memory.

DWORD	FLItemElem::GetGeneralPiercingItemID( const size_t Nth ) const
{
	return m_kPiercingOption.GetGeneralPiercingItemID( Nth );
}

const DWORD	FLPiercingOption::GetGeneralPiercingItemID( const size_t Nth ) const
{
	if( m_vecGeneralPiercing.empty() == false && m_vecGeneralPiercing.size() > Nth )
	{
		return m_vecGeneralPiercing.at( Nth );
	}

	return 0;
}

[jooodzszsz]

Originally Posted by cookie69

It is not a cast, if you pass a negative Nth to FLItemElem::GetGeneralPiercingItemIDthen then Nth will stay negative.
And later in m_kPiercingOption.GetGeneralPiercingItemID the check will be successful as m_vecGeneralPiercing.size() is always bigger than a negative value.
And later you do "return m_vecGeneralPiercing.at( Nth );" which will search a value at a negative index.

This will not at 100% crash the server but can corrupt the stack and maybe crash it later or write random data into its memory.

DWORD	FLItemElem::GetGeneralPiercingItemID( const size_t Nth ) const
{
	return m_kPiercingOption.GetGeneralPiercingItemID( Nth );
}

const DWORD	FLPiercingOption::GetGeneralPiercingItemID( const size_t Nth ) const
{
	if( m_vecGeneralPiercing.empty() == false && m_vecGeneralPiercing.size() > Nth )
	{
		return m_vecGeneralPiercing.at( Nth );
	}

	return 0;
}

const size_t is unsigned integer, it CANT be negative!