Summary

Checks-Effects-Interactions order was not respected in the deposit(IERC20 token, uint256 amount) function allowing the attacker to call deposit(IERC20 token, uint256 amount) of ThunderLoan till he drains the contract's funds

Vulnerability Details

Actors:
Attacker: the malicious LP who's going to perform the reentrancy attack.
Victim: Other LPs.
Protocol: The ThunderLoan contract itself.
Exploit Scenario:
Initial State: The ThunderLoan contract already contains liquidity thanks to other LPs.
Step 1: Attacker deposit a certain amount of token.
Step 2: Attacker calls the deposit(IERC20 token, uint256 amount) function with his malicious contract.
Step 3: When deposit(IERC20 token, uint256 amount) is called, the protocol makes the precious error of making an external call to the attacker's contract before updating the exchange rate AND actually depositing the funds through token.safeTransferFrom(msg.sender, address(assetToken), amount);.
Step 4: The attacker doesn't forget to add a receive() or fallback() function to his malicious contract that will keep on calling the function till he drains the protocol's contract from all its funds.

Impact

Outcome: The protocol will lose a lot of funds.
Implications: No more Liquidity

Tools Used

Manual review

Recommendations

• assetToken.mint(msg.sender, mintAmount);

• uint256 calculatedFee = getCalculatedFee(token, amount);

• assetToken.updateExchangeRate(calculatedFee);

• token.safeTransferFrom(msg.sender, address(assetToken), amount);

• uint256 calculatedFee = getCalculatedFee(token, amount);

• assetToken.updateExchangeRate(calculatedFee);

• token.safeTransferFrom(msg.sender, address(assetToken), amount);

• assetToken.mint(msg.sender, mintAmount);