Hack Fixes

07/16/2015 14:32 ディオニュソス#1

I don't really like hackers aiming for destroying every single server with little security issues, hence I'm going to release every single dupe/stack fix I find during the development of Memories of Madrigal.

Here are 3 to begin with, I'm going to update this thread accordingly.

Housing Crash:

In the function CHousing::IsSetupAble add

Code:

	if(housingInfo.bSetup)
	{
		if(pItemPropReq->dwItemKind3 != IK3_WALLPAPER && pItemPropReq->dwItemKind3 != IK3_CARPET)
		{
			if(!pWorld->VecInWorld(housingInfo.vPos))
			{
				return FALSE;
			}
		}
	}

below

Code:

	ItemProp* pItemPropReq = prj.GetItemProp( housingInfo.dwItemId );
	if( !pItemPropReq )
		return FALSE;

Bug type: Delayed crash
Issue: Trying to delete not spawned object
Further prevention: Check all AddObj calls return value for true and invalidate the object if false is returned.

BuyItem Dupe:

In the function CDPSrvr::OnBuyItem add

Code:

		long long lEntireCost = nNum * nCost;
		if(lEntireCost > INT_MAX)
		{
			return;
		}

below

Code:

#if __VER >= 11 // __MA_VER11_02
		if( pItemElem->m_dwItemId == II_SYS_SYS_SCR_PERIN )
			nCost = PERIN_VALUE;
#endif //__MA_VER11_02
		if( nCost < 1 )
			nCost = 1;

Bug type: Buy more items for less money
Issue: Int overflow

Consignment Stat Stack (WurstbrotQT Consignment):

In the function CConsignmentMng::AddItem add

Code:

	if(pVendor->m_Inventory.IsEquip(dwItemId))
	{
		return
	}

below

Code:

	if( nCount <= 0 || nCount > pItemElem->m_nItemNum )
		return false;

Bug type: Stat stack
Issue: Removing an equipped item

07/16/2015 15:06 xTwiLightx#2

Its nice to see that there are still people who are sharing fixes like these, not only the fix itself but also some short description what is causing the bug/crash and how to fix it.

I appreciate that a lot!

07/16/2015 15:53 Xylenu#3

I think there is something wrong:

Quote:

if(pItemPropReq->dwItemKind3 != IK3_WALLPAPER && pItemProp->dwItemKind3 != IK3_CARPET)
{
if(!pWorld->VecInWorld(housingInfo.vPos))
{
return FALSE;
}
}

Change the pItemProp to pItemPropReq ^^

07/16/2015 15:57 ディオニュソス#4

Quote:

Originally Posted by Xylenu

I think there is something wrong:
Change the pItemProp to pItemPropReq ^^

Yeah, I've already fixed that, sorry, the thread's been written with the phone and some information may or may not be lost.

I've also added a check to the housing crash to only check if the item is to be added.

Some further input about integer overflow:
Rather than let them happen I personally check whether an arithmetic operation is possible without an overflow since unsigned integer overflows result in undefined behaviour (C99 3.4.3/1).

I'm using the following utility functions to check whether the operation will overflow or not:

Code:

	class CUtils{
	public:
		template<class T>
		static bool CanAdd(const T & value, const T & toadd)
		{
			const T max = pow(static_cast<T>(2), static_cast<T>(sizeof(T) * 8 - 1)) - 1;

			ASSERT(value >= 0);
			ASSERT(toadd >= 0);

			if (value == 0 || toadd == 0)
				return true;

			return (max - toadd) >= value;
		}

		template<class T>
		static bool CanMul(const T & value, const T & factor)
		{
			// assuming signed T
			const T max = pow(static_cast<T>(2), static_cast<T>(sizeof(T) * 8 - 1)) - 1;

			ASSERT(value >= 0);
			ASSERT(factor >= 0);

			// divide by zero/fast exit
			if (factor == 0 || value == 0)
				return true;

			return (value <= max / factor);
		}

	private:
		template<class T>
#if _MSC_VER > 1800
		constexpr static T pow(const T base, unsigned const exponent)
#else
		static T pow(const T base, unsigned const exponent)
#endif
		{
			return (exponent == 0) ? 1 : (base * pow(base, exponent - 1));
		}
	};