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

Low Severity Issues

vinica_boy

L01: Wrong event emitted when calling settleAskTaker()

Summary

Incorrect event is emitted when calling settleAskTaker().
Instead of emitting SettledAskTaker, SettledBidTaker is emitted.

Vulnerability Details

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

On line #775 in PreMarkets.sol SettledBidTaker is emitted instead of SettledAskTaker.

Impact

Wrong event emitted can lead to inconsistency in off-chain related software.

Tools Used

Manual review.

Recommendations

Emit SettledAskTaker instead of SettledBidTaker.

L02: Fee on transfer ERC20 collateral tokens can lead to DoS when trying to transfer them

Summary

Checking for balance before and after transferring tokens is incompatible with ERC20 tokens that implement fee-on-transfer functionality. This is because fee-on-transfer tokens deduct a fee during the transfer process, causing the post-transfer balance to be less than expected. As a result, simple balance checks may incorrectly flag successful transfers as failures, leading to issues in handling such tokens correctly.

Vulnerability Details

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

_transfer() function in TokenManager.sol

function _transfer(
address _token,
address _from,
address _to,
uint256 _amount,
address _capitalPoolAddr
) internal {
uint256 fromBalanceBef = IERC20(_token).balanceOf(_from);
uint256 toBalanceBef = IERC20(_token).balanceOf(_to);
​
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();
}
}
}

Impact

Some tokens can lead to DoS of the protocol functionality.

Tools Used

Manual review.

Recommendations

Instead of strictly comparing fromBalanceAft with fromBalanceBef - _amount, calculate the actual amount transferred by subtracting fromBalanceAft from fromBalanceBef. This allows the function to accommodate fee-on-transfer tokens, where the actualAmountTransferred might be less than the _amount.

Based on the protocol team, accepted fee can be adjusted.

Updates

Lead Judging Commences

0xnevi Lead Judge about 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)

finding-PreMarkets-settleAskTaker-wrong-event

Valid low, wrong event emitted

Support

FAQs

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