Findings

Issue 01

Type	Severity	Location	Status
Logical Issue	Informational	liquidatePosition	✔️ Fixed

Description

There is no liquidator in liquidiatePosition, according to the line comments only liquidator rule can call liquidatePosition.

Issue 03

Type	Severity	Location	Status
Logical Issue	Medium	_openPosition	✔️ Fixed

Description

IUniMexPool(pool).borrow(values[0]);

If the token in the pool has fees on transfer, the amount received to the ApeMexMargin contract will be less than expected due to fee deduction. Your code doesn't support this kind of scenario because the code is trying to swap value[0] amount.

swap = swapTokens(baseToken, quoteToken, values[0]);

The borrower will get the fees for free.

Issue 04

Type	Severity	Location	Status
Owner Privileges	Medium	setThresholdGasPrice	❌ Not Fixed

Description

The owner of the contract can set thresholdGasPrice which sets the current gas price. The problem is that the owner of the contract can set the threasholdGasPrice to be really high in that it would maximize the fees on autoClose , autoOpen , and liquidatePosition at the expense of the borrower's commitment.

Issue 05

Type	Severity	Location	Status
Logical Issue	Informational	autoClose	✔️ Fixed

Description

According to the documentation, validBefore should be in block height, however, it is compared to block.timestamp.

Issue 06

Type	Severity	Location	Status
Gas Optimization	Informational	closeShort , closeLong	❌ Not Fixed

Description

The gas consumed by this function can be reduced by using getAmountsIn function to calculate how much from the commitment needed to be swapped in order to cover the remainder.

Issue 07

Type	Severity	Location	Status
Logical Issue	High	_closeLongInformational	✔️ Fixed

Description

uint256 amount = isProfit ? swap.sub(owed) : commitment.sub(owed.sub(swap)
2 transferToPool(pool, BASE_TOKEN_ADDRESS, owed);
3 transferFees(fees, pool);
4 transferEscrowToUser(position.owner, isProfit ? position.owner : a
5 commitment);
6 transferToUser(position.owner, amount.sub(fees));

Consider the following scenario - the owed amount is 10 and the swap is 12, fees are 3, the position will be considered as profit (isProfit will be true). However, the amount is 12 - 10 = 2, so amount.sub(fees) will underflow Note that the require() still passes - commitment is enough to be larger than owed + fees.

Issue 08

Type	Severity	Location	Status
Logical Issue	High	_closeLong	✔️ Fixed

Description

Using the same example of Issue 06, commitment should be used to cover the fees, but the user always receives all commitments from the escrow

Issue 09

Type	Severity	Location	Status
Logical Issue	Informational	_openPosition	⚠️ Acknowledged

Description

fees = swapTokens(token, BASE_TOKEN_ADDRESS, fees); // convert fees to ETH

According to the comment, it seems that this function supposes to convert the fees to ETH, however according to the logic the fees are swapped to BUSD.

Issue 10

Type	Severity	Location	Status
Logical Issue	Informational	liquidatePosition , canLiquidate	✔️ Fixed

Description

According to the documentation liquidity margin is supposed to be 11%, however, it is 10%.

LIQUIDATION_MARGIN / MAG = 1.1 which is 10%, but doc says 11%

Issue 11

Type	Severity	Location	Status
Logical Issue	Informational	_openPosition	✔️Fixed

Description

Your code doesn't support tokens with fees on transfer. When you call borrow function you expect that value[0] will be the amount that would be transferred from the pool, however if there are fees on transfer you will receive less than value[0] which means borrowers will "enjoy" the fees for free.