[Release]Security Fixes

05/12/2016 19:30 Zerux#1
Dear Elitepvpers,
I decided to release some exploits before anyone uses them against any servers.
Most of them are not Public as far as I know, most of them need a bit of advanced knowledge to exploit:

SQL-Injection Guild [Function: CDPSrvr::OnQuerySetGuildName]:
In this case there are 2 Bugs which make this attack even possible on most of the servers.
Problem 1:
Quote:
There is no check if the item that is used is the "Change Guild-Name" item
Problem 2:
Quote:
The developers validate the String that contains the name ("lpszGuild") but even if this check fails they forgot to return.
Solution[Define for both fixes is __GUILD_RENAME_FAKE]:



SQL-Injection Name [Function: CDPSrvr::OnQuerySetPlayerName]:
This is similar to the one above.
Problem :
Quote:
The developers validate the String that contains the name ("lpszPlayer") but even if this check fails they forgot to return.
Solution[Define for the fix is __CHAR_RENAME_INVALID]:



Buffer Overflow [Function: CProject::IsInvalidName]:
This is a critical bug, but most of the "flyff hackers" would not be able to abuse this other than just crashing the server.
Problem :
Quote:
The critical line of this is
Quote:
strcpy( pszName, szName );
"pszName" is a local buffer of size 64 but szName can be up to 100 characters long.
If szName exeeds the limit of 64 ,the stack canary is overwritten and therefore an error will be raised resulting in a servercrash. There are 2 ways to fix this: Either increasing the buffersize up to 100 or preventing the buffer overflowing
Solution[Define for the fix is __BUFFEROVERDLOW_NAMECHECK]:



Array out of bounds [Function: CItemUpgrade::RemovePetVisItem]:
This can result in a servercrash or even stathacks and more.
Problem :
Quote:
I will keep this simple. There is no check of the value of "nPosition" so we can easily get out of bounds
Solution[Define for the fix is __PET_PIERCING_BOUNDS]:


UPDATE 1 (15.05.2016)
Array out of bounds [Function: CExchange::ResultExchange]:
This can result in a servercrash or even dupes and more.
Problem :
Quote:
Similar to the one above. There is no check of the value of "nListNum" so we can easily get out of bounds as if its "nListNum < 0" it will pass all checks
Solution[Define for the fix is __EXCHANGE_NEGATIVE]:


UPDATE 2 (02.04.2018)
Nullpointer [Function: CDPSrvr::OnTransformItem]:
This results in a servercrash.
Problem :
Quote:
ITransformer::Transformer only returns a valid Object if the argument is 0 which is for EggTransformation and otherwise NULL which leads to de-referencing a nullpointer (if you actively use Assertions you might want to add a check in the Transformer function itself)
Solution[Define for the fix is __INVALID_TRANSFORM]:


UPDATE 3 (09.10.2018)
Out of Memory [Function: CDPSrvr::OnRainbowRaceMiniGamePacket]:
This can result in a servercrash .
Problem :
Quote:
When serializing __MINIGAME_EXT_PACKET the size of the vector is not checked(nSize) which allows people to create arbitrary size vectors.
Solution[This is NOT a perfect fix but as far as i know the maximum size used in this system is 3. Define for the fix is __MINIGAME_SIZE_VALIDATE]:

Array out of bounds[Function: CMiniGame::Bet_FiveSystem]:
This can result in a servercrash or even dupes and more.
Problem :
Quote:
The developers messed up. They validated the index (nBetNum) but too late.
Solution[Define for the fix is __MINIGAME_EARLY_CHECK]:



I will only be releasing bugs that affect the availability of the server. I won't be releasing fixes for anything that is not causing a DoS.

If your server gets crashed by someone and the fix is not public, please don't hesitate to contact me, I'm happy to help :)

Zerux
05/15/2016 13:55 Zerux#2
Added another fix which I forgot in the initial thread :)
Quote:
UPDATE 1 (15.05.2016)
Array out of bounds [Function: CExchange::ResultExchange]
04/02/2018 13:08 Zerux#3
Another bug I came across after I was asked to check a specific part of the code ;)
Quote:
UPDATE 2 (02.04.2018)
Nullpointer [Function: CDPSrvr::OnTransformItem]
10/09/2018 18:43 Zerux#4
Another one. More coming soon (Fixes for fixes of other Devs ;) )
Quote:
UPDATE 3 (09.10.2018)
Out of Memory [Function: CDPSrvr::OnRainbowRaceMiniGamePacket]
Quote:
Array out of bounds[Function: CMiniGame::Bet_FiveSystem]
10/11/2018 04:35 Rhyder`#5
Return nBetPenya also when get < 0 to avoid crash

Code:
if(nBetPenya  < 0)
return FIVESYSTEM_FAILED;
10/11/2018 12:36 Zerux#6
Quote:
Originally Posted by Rhyder` View Post
Return nBetPenya also when get < 0 to avoid crash

Code:
if(nBetPenya  < 0)
return FIVESYSTEM_FAILED;
This is already present in the official code:

Quote:
// 입찰금은 0보다 작을 수 없다.
if(nBetPenya < 0)
return FIVESYSTEM_FAILED;