First Flight #3: Thunder Loan

In ThunderLoan::initialize, the Ownable initializer is called to set the owner of the contract as the address calling initialize(). Since the deployer of the contract will need to call initialize() after the contract has been deployed, the function call can be front-run meaning that another address will become the owner of the contract rather than the deployer.

Vulnerability details

In ThurderLoan::initialize, the owner of the contract is set using the __Ownable_init Ownable initializer on line 140:

Unlike constructors, initializers have to be called after the contract has been deployed. This means that after the ThunderLoan contract has been deployed, an attacker can call initialize() which would set the owner as this attacking address. This means that the owner is now not the deployer of the contract as desired.

Impact

Only the owner of the contract can call functions that have the onlyOwner modifier.

These functions include:

• setAllowedToken(): sets which tokens are allowed to be loaned and deposited

• updateFlashLoanFee(): updates the fee but this function has a check that means the fee can only increase.

• _authorizeUpgrade(): allow the implementation to be upgraded. This is problematic since the attacker can maliciously modify all functions.

Since these capabilities carry risks associated with centralization, as detailed in the issue: "Centralization risk given the owner has privileges to upgrade the implementation and modify the fees", the deployer of the contract would need to re-deploy the contract which would waste the gas and deployment costs, and inform users of the newly deployed contract if the initially deployed contract had been publicized. This is therefore a low-severity vulnerability since the impact is low and the likelihood is low.

Recommended mitigation

Use a script to deploy and initialize the contract in the same transaction using a deployer contract and transfer ownership to the desired address or pass as an additional param to initialize.