transferStakedSaltFromAirdropToUser will potentially grant users with no exchange access staked salt

[c4-bot-2]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/staking/Staking.sol#L130-L138

Vulnerability details

Bug Description

The Salty protocol's staking functionality, managed by the Staking.sol contract, is designed to restrict XSalt (staked salt) possession to users with access to the exchange. This restriction is enforced in the stakeSALT() function, which requires users to have exchange access to stake salt.

function stakeSALT( uint256 amountToStake ) external nonReentrant
    {
    require( exchangeConfig.walletHasAccess(msg.sender), "Sender does not have exchange access" );

However, this access check is overlooked in the airdrop distribution process. The transferStakedSaltFromAirdropToUser() function, responsible for transferring XSalt from the airdrop to users, does not verify if the recipient is permitted to access the exchange.

// Send xSALT from the Airdrop contract to a user
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 );
    _increaseUserShare( wallet, PoolUtils.STAKED_SALT, amountToTransfer, false );

    emit XSALTTransferredFromAirdrop(wallet, amountToTransfer);
    }

Impact

This oversight allows users without exchange access (e.g., users from blacklisted countries) to hold XSalt, bypassing the protocol's intended restrictions.

Proof of Concept

The issue can be demonstrated as follows:

• Users are whitelisted for the airdrop.
• During the airdrop claim process, a user without exchange access (like Alice in the provided POC) successfully claims XSalt.
• This scenario contradicts the protocol's intention that only users with exchange access should hold XSalt.

function testClaimAirdropNoAccess() external {
    // Whitelist
    whitelistAlice();

    // Approve the distribution to allow claiming
    vm.prank(address(bootstrapBallot));
    initialDistribution.distributionApproved();
    assertTrue(airdrop.claimingAllowed(), "Claiming should be allowed for the test");

    // Claim airdrop
    vm.prank(alice);
    airdrop.claimAirdrop();

    // Verify that Alice successfully claimed
    assertTrue(airdrop.claimed(alice), "Alice should have successfully claimed the airdrop");
    }

The test must be added to /launch/tests/Airdrop.t.sol and can be run by calling COVERAGE="yes" NETWORK="sep" forge test -vvvv --rpc-url https://rpc.sepolia.org --match-test "testClaimAirdropNoAccess".

The provided test case needs to be modified by commenting out grantAccessAlice(); in Line 115 of the file to simulate the situation where a user without exchange access successfully claims the airdrop.

Tools Used

Manual Review

Recommended Mitigation Steps

To rectify this issue, a check should be added in the transferStakedSaltFromAirdropToUser() function to ensure that the recipient has access to the exchange. This can be implemented as follows:

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

    require( exchangeConfig.walletHasAccess(wallet), "Sender does not have exchange access" );

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

    emit XSALTTransferredFromAirdrop(wallet, amountToTransfer);
    }

By implementing this check, the protocol will enforce its access restrictions consistently across both staking and airdrop distribution, ensuring that only eligible users can hold XSalt.

Assessed type

Invalid Validation

[c4-judge]

Picodes marked the issue as primary issue

[c4-sponsor]

othernet-global (sponsor) disputed

[othernet-global]

In order to receive the airdrop, users will need to vote on the BootstrapBallot and satisfy the same geo restrictions as for general exchange access.

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)