Some SALT rewards can get stuck in the Airdrop contract.

[c4-bot-2]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/Staking.sol#L134

Vulnerability details

When the protocol is launched, 3 million SALT is allocated to the xSALT staking contract and 5 million SALT can be claimed by the airdrop participants.

So 1% from 3 million(30k) will likely be available for claim in the xSALT contract right after the protocol launches. And because the circulating supply will initially be so small, it is likely that the first depositor in the xSALT contract will be someone claiming their airdrop.

The airdrop contract first stakes the SALT to be airdropped and then it unstakes it and stakes again but for the user. The problem here is that the when existingTotalShares != 0 the virtualRewards are not added so for example:

1. The protocol launches and 30k SALT rewards are sent to the xSALT staking contract
2. The first depositor is someone claiming their airdrop and the Airdrop contract will call transferStakedSaltFromAirdropToUser():

function transferStakedSaltFromAirdropToUser(address wallet, uint256 amountToTransfer) external {
        require( msg.sender == address(exchangeConfig.airdrop()), "Staking.transferStakedSaltFromAirdropToUser is only callable from the Airdrop contract" );

        _decreaseUserShare( msg.sender, PoolUtils.STAKED_SALT, amountToTransfer, false );

And as you can see _decreaseUserShare() is called with msg.sender, so 30k SALT will be transferred to the Airdrop contract and will be stuck there forever because when decreasing shares we are also claiming the rewards and the virtualRewards are not set so the Airdrop contract just gets all the rewards.

Impact

SALT gets transferred to the Airdrop contract and will be stuck there forever.

Proof of Concept

Add this to Staking.t.sol

function testSaltStuckInAirdrop() public {
        address airdrop = address(exchangeConfig.airdrop());

        vm.prank(DEPLOYER);
        salt.transfer(address(initialDistribution), 30000 ether);

        bytes32[] memory poolIDs = new bytes32[](1);
        poolIDs[0] = PoolUtils.STAKED_SALT;

        AddedReward[] memory addedRewards = new AddedReward[](1);
        addedRewards[0] = AddedReward( PoolUtils.STAKED_SALT, 30000 ether );

        vm.startPrank(address(initialDistribution));
        salt.approve( address(staking), type(uint256).max );
        staking.addSALTRewards(addedRewards);
        vm.stopPrank();

        vm.prank(DEPLOYER);
        salt.transfer(airdrop, 10 ether);

        uint256 amountToTransfer = 10 ether;

        vm.startPrank(airdrop);
        salt.approve(address(staking), type(uint256).max);
        staking.stakeSALT(amountToTransfer);
        staking.transferStakedSaltFromAirdropToUser(alice, amountToTransfer);
        vm.stopPrank();

        console.log(salt.balanceOf(airdrop));
    }

Tools Used

Foundry

Recommended Mitigation Steps

Add a function in Airdrop to withdraw this SALT

Assessed type

Other

[c4-judge]

Picodes marked the issue as primary issue

[c4-sponsor]

othernet-global (sponsor) disputed

[othernet-global]

This is acceptable and any SALT in the Airdrop contract will be treated as not being part of the circulating supply.

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)