15,000 USDC
View results
Submission Details
Severity: medium
Valid

Protocol doesn't work properly for token that have fee on transfer

Summary

When users deposit and redeem collateral token, it always assumed that amountCollateral is always equal to the transferred or received amount, this will cause issue if the collateral used has fee on transfer, breaking the protocol accounting.

Vulnerability Details

depositCollateral assume that amountCollateral that added to s_collateralDeposited equal to collateral transferred from users :

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L149-L161

function depositCollateral(address tokenCollateralAddress, uint256 amountCollateral)
public
moreThanZero(amountCollateral)
isAllowedToken(tokenCollateralAddress)
nonReentrant
{
s_collateralDeposited[msg.sender][tokenCollateralAddress] += amountCollateral;
emit CollateralDeposited(msg.sender, tokenCollateralAddress, amountCollateral);
bool success = IERC20(tokenCollateralAddress).transferFrom(msg.sender, address(this), amountCollateral);
if (!success) {
revert DSCEngine__TransferFailed();
}
}

_redeemCollateral also do the same thing :

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/main/src/DSCEngine.sol#L281-L291

function _redeemCollateral(address from, address to, address tokenCollateralAddress, uint256 amountCollateral)
private
{
s_collateralDeposited[from][tokenCollateralAddress] -= amountCollateral;
emit CollateralRedeemed(from, to, tokenCollateralAddress, amountCollateral);
bool success = IERC20(tokenCollateralAddress).transfer(to, amountCollateral);
if (!success) {
revert DSCEngine__TransferFailed();
}
}

Impact

This will cause internal accounting inaccurate and eventually break the protocol

Tools Used

Manual review

Recommendations

Consider to check before and after transfer balance, and used the diff for accounting for updating s_collateralDeposited

Support

FAQs

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