Foundry DeFi Stablecoin CodeHawks Audit Contest
Cyfrin
DeFiFoundry
15,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Fee on transfer tokens will not behave as expected

bauchibred

Summary

Integrating fee-on-transfer tokens would completely break DSC's internal accounting

Vulnerability Details

The current implementation of the DSC protocol doesn't work with fee-on-transfer as collateral tokens.
When a fee is charged on a transfer of tokens in Solidity, it is important to check the balance of the sender's account before and after the transfer to ensure that the fee has been correctly deducted from the sender's balance.

Take a look at DSCEngine.sol#L144-L161

/*
* @notice follows CEI
* @param tokenCollateralAddress The address of the token to deposit as collateral
* @param amountCollateral The amount of collateral to deposit
*/
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();
}
}

As seen no check is performed to really check if the amount of tokens received are what was sent note that since this check is not performed, it can result in an accounting error where the depositor's collateral value is overstated, leading to potential issues with record keeping, reporting, and reconciliation.

For example the getAccountCollateralValue() function

function getAccountCollateralValue(address user) public view returns (uint256 totalCollateralValueInUsd) {
// loop through each collateral token, get the amount they have deposited, and map it to
// the price, to get the USD value
for (uint256 i = 0; i < s_collateralTokens.length; i++) {
address token = s_collateralTokens[i];
uint256 amount = s_collateralDeposited[user][token];
totalCollateralValueInUsd += getUsdValue(token, amount);
}
return totalCollateralValueInUsd;
}

When this is called would iterate over all the tokens added in the user's collateral array, but user's collateral value would be easily overstated since the amount of registered collateral is not really what's been registered in the contract

Impact

Break of internal accounting, in the case of the depositCollateral function of DSCEngine.sol, the contract directly registers the amountCollateral provided and not the one received.

Tool Used

Manual Audit

Recommendation

Check the balances before and after any transfer for tokens so as to ensure correct accurate accounting.

Support

FAQs

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