Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

All main functionality of the `PreMarkets` contract could be bricked if USDC tokens start charging a fee.

mikebello

Summary

The main functions of the Premarkets contract createOffer, createTaker, listOffer, relistOffer would be bricked if USDC starts charging a fee.

Vulnerability Details

The createOffer, createTaker, listOffer, relistOffer function in the Premarkets contracts calls the tillIn function in the `TokenManager` contract to handle the token transfers between the sender, the capital Pool and the Token manager, this function uses an internal _transfer function to handle the accounting of these transfers, this internal function calculates the balance of the `from` and `to` addresses before and after the transfer and then validates and reverts the transaction if the balances after is different from the balances before minus the amount transferred.

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L96

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L356

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L515

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L831

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L164-L284

function _transfer(address _token, address _from, address _to, uint256 _amount, address _capitalPoolAddr)
internal
{
uint256 fromBalanceBef = IERC20(_token).balanceOf(_from); // msg.sender
uint256 toBalanceBef = IERC20(_token).balanceOf(_to); // balance del capital pool (llamada desde tillIn function)
​
if (_from == _capitalPoolAddr && IERC20(_token).allowance(_from, address(this)) == 0x0) {
ICapitalPool(_capitalPoolAddr).approve(address(this));
}
_safe_transfer_from(_token, _from, _to, _amount);
​
uint256 fromBalanceAft = IERC20(_token).balanceOf(_from);
uint256 toBalanceAft = IERC20(_token).balanceOf(_to);
if (fromBalanceAft != fromBalanceBef - _amount) {
revert TransferFailed();
}
​
if (toBalanceAft != toBalanceBef + _amount) {
revert TransferFailed();
}
}

This design won't work with ERC20s that currently implement or may implement fees on transfers in the future, like USDT and USDC, as USDC is an upgradeable token, they can implement fees on transfers in the future, and as the protocol is using USDC and its intention is to work with every ERC20 token, this will brick the main functionality of the PreMarkets contract because the _transfer function will revert when the function checks if the balances after are equal to the balances before minus the amount transferred.

if (fromBalanceAft != fromBalanceBef - _amount) {
revert TransferFailed();
}
​
if (toBalanceAft != toBalanceBef + _amount) {
revert TransferFailed();
}

Impact

All the main functions of the PreMarket contract will be bricked.

Tools Used

Manual Code Review

Recommendations

update the last if's of the _transfer function to account for fee on transfer tokens.

Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-TokenManager-FOT-Rebasing

Valid medium, there are disruptions to the ability to take market actions. The following functions will be disrupted without the possibiliy of reaching settlement, since the respective offers cannot be created/listed regardless of mode when transferring collateral token required to the CapitalPool contract or when refunding token from user to capital pool during relisting. So withdrawal is not an issue - `createOffer()` - reverts [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L96-L102) - `listOffer()` - reverts [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L355-L362) - `relistOffer()` - reverts [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L515-L521) - `createTaker()` - reverts [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L831-L836) I believe medium severity is appropriate although the likelihood is high and impact is medium (only some level of disruption i.e. FOT tokens not supported and no funds at risk)

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!