Zaros Part 1

Lines of code

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/TradingAccountBranch.sol#L369

Impact

According to the netspec, the amount provided when calling withdrawMargin is the UD60x18 amount. Nevertheless, the amount is still converted to UD60x18 which, for tokens with less than 18 decimals, increased the amount by a magnitude of 18 - decimal of the token. This will result in the revert of the call or the withdrawal of way to many collateral.

Proof of Concept

According to the netspec, the amount which is provided to the function withdrawMargin is a UD60x18 value.

Nevertheless, the amount is upscaled to 18 decimals using convertTokenAmountToUd60x18():

convertTokenAmountToUd60x18() takes the amount and scales it up by the difference of decimals of the token compared to 18 decimals:

This means that any collateral which has less than 18 decimals will be upscaled. E.g. USDT with 6 decimals will be upscaled by 1e12. This results in an overinflated amount which is used to determine if the user deposited enough collateral which will fail and in turn revert the function call.

Recommended Mitigation Steps

Since the provided amount is already with 18 decimals there is not need to scale it further. Instead simply convert the provided uint256 to a UD60x18: