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

After you upgrade to UpgradedThunderLoan.sol, functions that have the OnlyThunderLoan modifier will revert, which will brick multiple critical functions

Summary

MockFlashLoanReceiver.sol and AssetToken.sol have a modifier onlyThunderLoan which restricts the calling of certain functions to only the ThunderLoan contract, including executeOperation in the former and mint and burn in the latter. onlyThunderLoan was set to the address for ThunderLoan.sol when MockFlashLoanReceiver.sol and AssetToken.sol were initially deployed, and there is no way to upgrade this address when ThunderLoanUpgraded.sol is deployed. executeOperation is critical for executing flash loans and mint and burn are critical for deposits and redemptions. As a result of this, you have bricked your entire protocol.

Vulnerability Details

Here in AssetToken.sol is the modifier onlyThunderLoan as well as the constructor where ThunderLoan's address is set permanently. It is the same in MockFlashLoanReceiver.sol. In this example, the address is labeled immutable - you don't want this to be immutable if you plan to upgrade your contract.

modifier onlyThunderLoan() {
if (msg.sender != i_thunderLoan) {
revert AssetToken__onlyThunderLoan();
}
_;
}
/*//////////////////////////////////////////////////////////////
FUNCTIONS
//////////////////////////////////////////////////////////////*/
constructor(
address thunderLoan,
IERC20 underlying,
string memory assetName,
string memory assetSymbol
)
ERC20(assetName, assetSymbol)
revertIfZeroAddress(thunderLoan)
revertIfZeroAddress(address(underlying))
{
i_thunderLoan = thunderLoan;
i_underlying = underlying;
s_exchangeRate = STARTING_EXCHANGE_RATE;
}

Impact

The core functions of your protocol are bricked. Whenever someone tries to call deposit, redeem, or flashloan, it will revert.

Tools Used

Manual review

Recommendations

Make address thunderLoan a state variable and allow onlyOwner to set ThunderLoan's address in a function like the following. The owner should stay the same from the initial contract to the next.

function setThunderLoan(address thunderLoan) public onlyOwner {
s_thunderLoan = thunderLoan;}
Updates

Lead Judging Commences

0xnevi Lead Judge
about 2 years ago
0xnevi Lead Judge about 2 years ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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