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

Deployment script indicates there's a missing proxy initialization

Summary

The deployment script is deploying an ERC1967Proxy but then fails to initialize it.

Vulnerability Details

Due to the fact that src/protocol/ThunderLoan::initialize and src/upgradedProtocol/ThunderLoanUpgraded::initialize are all unprotected external methods, anyone can frontrun the deployment and initialize the proxy with a address(0) as the tswapAddress thereby making this instance of the application completely useless.
Combining the current state of the deployment script as is with a bot and it causes a denial of service to the protocol authors as well as users as it won't let a functioning instance of the application be deployed.

POC

Put this piece of code in `test/unit/ThunderLoanTest.t.sol`
function testWrongInitializationCannotDeposit() public setAllowedToken {
tokenA.mint(liquidityProvider, AMOUNT);
vm.startPrank(liquidityProvider);
tokenA.approve(address(thunderLoan), AMOUNT);
vm.expectRevert();
thunderLoan.deposit(tokenA, AMOUNT);
vm.stopPrank();
}
Put the following changes in `test/unit/BaseTest.t.sol::setup`
function setUp() public virtual {
thunderLoan = new ThunderLoan();
mockPoolFactory = new MockPoolFactory();
weth = new ERC20Mock();
tokenA = new ERC20Mock();
mockPoolFactory.createPool(address(tokenA));
proxy = new ERC1967Proxy(address(thunderLoan), "");
thunderLoan = ThunderLoan(address(proxy));
+ vm.prank(address(20));
+ thunderLoan.initialize(address(0));
- thunderLoan.initialize(address(mockPoolFactory));
}

and run in the terminal forge test --mt testWrongInitializationCannotDeposit -vvv

Impact

By initializing the proxy with address(0) as the pool factory address, this makes it impossible to deposit/redeem as the call to ThunderLoan::getCalculatedFee or ThunderLoanUpgraded::getCalculatedFee will always revert at getPriceInWeth call.
This line particularly.
address swapPoolOfToken = IPoolFactory(s_poolFactory).getPool(token);
Let's note that, because the initialize function was called by the attacker, he effectively seized the ownership of the protocol from the protocol authors and can now do only owner stuff like say set an exhorbitantly high flashloan fee making the protocol useless as well.

Tools Used

Manual review

Recommendations

put the following change in `script/DeployThunderLoan.s.sol`
function run() public {
vm.startBroadcast();
ThunderLoan thunderLoan = new ThunderLoan();
- new ERC1967Proxy(address(thunderLoan), "");
+ ERC1967Proxy proxy = new ERC1967Proxy(address(thunderLoan), "");
+ thunderLoan = ThunderLoan(address(proxy));
+ thunderLoan.initialize(address_of_pool_factory);
vm.stopBroadcast();
}
Updates

Lead Judging Commences

0xnevi Lead Judge
over 1 year ago
0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Front-running initializers

Support

FAQs

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