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

Fee-on-transfer/Rebasing tokens are not properly accounted for

madalad

Summary

When transferring tokens from another account to address(this), the amount of tokens received by the contract is not necessarily equal to the amount parameter. This can be the case for:
- fee on transfer tokens: e.g. PAXG
- a fee is taken out of each transfer from the amount and sent to a
predetermined fee recipient, meaning the to address receives fewer
tokens
- rebasing tokens: e.g. LDO
- user token balances are algorithmically altered automatically,
meaning balances change without any action being taken

If it is assumed that a contracts balance of a particular token is equal to the amount parameter used in transferFrom, this can be voilated.

Vulnerability Details

Mappings s_collateralDeposited and s_DSCMinted assume that the amount of tokens received by the contract is equal to the amount value passed to the transferFrom call, which is not guaranteed.

File: src\DSCEngine.sol
​
149: function depositCollateral(address tokenCollateralAddress, uint256 amountCollateral)
150: public
151: moreThanZero(amountCollateral)
152: isAllowedToken(tokenCollateralAddress)
153: nonReentrant
154: {
155: s_collateralDeposited[msg.sender][tokenCollateralAddress] += amountCollateral; // @audit may receive fewer than amountCollateral tokens
156: emit CollateralDeposited(msg.sender, tokenCollateralAddress, amountCollateral);
157: bool success = IERC20(tokenCollateralAddress).transferFrom(msg.sender, address(this), amountCollateral);
158: if (!success) {
159: revert DSCEngine__TransferFailed();
160: }
161: }
File: src\DSCEngine.sol
​
272: function _burnDsc(uint256 amountDscToBurn, address onBehalfOf, address dscFrom) private {
273: s_DSCMinted[onBehalfOf] -= amountDscToBurn; // @audit may receive fewer than amountDscToBurn tokens
274: bool success = i_dsc.transferFrom(dscFrom, address(this), amountDscToBurn);
275: // This conditional is hypothtically unreachable
276: if (!success) {
277: revert DSCEngine__TransferFailed();
278: }
279: i_dsc.burn(amountDscToBurn);
280: }

Impact

In the case of fee-on-transfer tokens, the contracts balance would be lower than expected. This leads to the contract becoming slightly undercollateralized, which may cause a loss of funds for certain users. For example, if everyone wanted to redeem their collateral, the final user to call redeem will receive fewer tokens than expected.

Tools Used

Manual review

Recommendations

Implement an allowlist for ERC20 tokens, or redesign functions to account for obscure tokens

Support

FAQs

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