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

transferFrom methods need allowance from owner of erc20 tokens to be used , but this is not available in `depositCollateral()`

0xsmartcontract

Summary

Both SafeTransferFrom and transferFrom functions in the ERC-20 standard require the sender to have an allowance for the ERC-20 tokens they are attempting to transfer.

The safeTransferFrom and transferFrom functions moves amount tokens from sender to recipient using the allowance mechanism. amount is then deducted from the caller’s allowance.

Vulnerability Details

OZ docs-- https://docs.openzeppelin.com/contracts/2.x/api/token/erc20#IERC20-transferFrom-address-address-uint256-

src/DSCEngine.sol:
148 */
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;
156: emit CollateralDeposited(msg.sender, tokenCollateralAddress, amountCollateral);
// ==> @audit doesn't call `approve` before transferFrom is used
157: bool success = IERC20(tokenCollateralAddress).transferFrom(msg.sender, address(this), amountCollateral);
158: if (!success) {
159: revert DSCEngine__TransferFailed();
160: }
161: }
162

Impact

Without an allowance, the transfer will fail, and the transaction would be reverted.

Tools Used

Manuel Code Review

Recommendations

Call approve before the safeTransferFrom method attempts to transfer funds

Support

FAQs

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