Beginner FriendlyFoundryDeFiOracle
100 EXP
View results
Submission Details
Severity: medium
Invalid

Depositor needs to approve deposit to send their tokens before safeTransferFrom is called

Summary

The deposit function uses safeTransferFrom to transfer the underlying tokens to be flashloaned to the asset token contract. Because the ThunderLoan contract is sending the depositor's tokens to the asset token contract on the user's behalf, the user needs to approve the contract to transfer their tokens before calling safeTransferFrom().

Vulnerability Details

function deposit(
IERC20 token,
uint256 amount
) external revertIfZero(amount) revertIfNotAllowedToken(token) {
AssetToken assetToken = s_tokenToAssetToken[token];
uint256 exchangeRate = assetToken.getExchangeRate();
uint256 mintAmount = (amount * assetToken.EXCHANGE_RATE_PRECISION()) /
exchangeRate;
emit Deposit(msg.sender, token, amount);
assetToken.mint(msg.sender, mintAmount);
uint256 calculatedFee = getCalculatedFee(token, amount);
assetToken.updateExchangeRate(calculatedFee);
token.safeTransferFrom(msg.sender, address(assetToken), amount);
}

Test

I wrote this test to show that you can't successfully deposit without approve....it reverts due to insufficient allowance:

function testCantDepositWithoutApproval() public setAllowedToken {
vm.startPrank(liquidityProvider);
tokenA.mint(liquidityProvider, DEPOSIT_AMOUNT);
vm.expectRevert();
thunderLoan.deposit(tokenA, DEPOSIT_AMOUNT);
vm.stopPrank();
}

Impact

Deposits will fail. In the unit tests, we had to call approve before being able to successfully deposit, but once this protocol is live, that needs to be part of the deposit function.

Tools Used

Manual review

Recommendations

Add approve before safeTransferFrom:

token.approve(address(this), amount);
Updates

Lead Judging Commences

0xnevi Lead Judge
over 1 year ago
0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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