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 ThunderLoanUpgraded 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 ThunderLoanUpgraded contract itself.
Exploit Scenario:
Initial State: The ThunderLoanUpgraded 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 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 all its funds.
Implications: No more liquidity

Tools Used

Manual review

Recommendations

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

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

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

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