Lines of code

https://github.com/code-423n4/2024-05-olas/blob/main/registries/contracts/staking/StakingToken.sol#L115-L128 https://github.com/code-423n4/2024-05-olas/blob/main/registries/contracts/staking/StakingBase.sol#L482-L511 https://github.com/code-423n4/2024-05-olas/blob/main/registries/contracts/staking/StakingBase.sol#L805-L872 https://github.com/code-423n4/2024-05-olas/blob/main/registries/contracts/staking/StakingToken.sol#L104-L111

Vulnerability details

Impact

According to https://github.com/code-423n4/2024-05-olas?tab=readme-ov-file#erc20-token-behaviors-in-scope, fee-on-transfer tokens are in scope and should be supported by this protocol. When this kind of token is stakingToken, calling the following deposit function to deposit funds for staking would increase balance and availableRewards by amount but the balance of such token held by the corresponding StakingToken contract would be increased by a value that equals such amount minus the fee of such token transfer. In this situation, the corresponding StakingToken contract's balance of such token is less than balance or availableRewards.

https://github.com/code-423n4/2024-05-olas/blob/main/registries/contracts/staking/StakingToken.sol#L115-L128

    function deposit(uint256 amount) external {
        // Add to the contract and available rewards balances
        uint256 newBalance = balance + amount;
        uint256 newAvailableRewards = availableRewards + amount;

        // Record the new actual balance and available rewards
        balance = newBalance;
        availableRewards = newAvailableRewards;

        // Add to the overall balance
        SafeTransferLib.safeTransferFrom(stakingToken, msg.sender, address(this), amount);
        ...
    }

After the first deposit, both balance and availableRewards are the same so it is possible that the reward accumulates to equal such availableRewards, which also equals such balance. When the accumulated reward equals balance, calling the following _claim or unstake function, which further calls the _withdraw function below, would reduce balance to 0 and attempt to transfer such accumulated reward from the corresponding StakingToken contract to the service owner. However, because the StakingToken contract's balance of such fee-on-transfer token is less than balance, such transfer reverts. As a result, the service owner fails to receive the reward that is entitled to her or him.

https://github.com/code-423n4/2024-05-olas/blob/main/registries/contracts/staking/StakingBase.sol#L482-L511

    function _claim(uint256 serviceId, bool execCheckPoint) internal returns (uint256 reward) {
        ServiceInfo storage sInfo = mapServiceInfo[serviceId];
        ...
        // Get the claimed service data
        reward = sInfo.reward;
        ...
        // Transfer accumulated rewards to the service multisig
        // Note that the reentrancy is not possible since the reward is set to zero
        address multisig = sInfo.multisig;
        _withdraw(multisig, reward);
        ...
    }

https://github.com/code-423n4/2024-05-olas/blob/main/registries/contracts/staking/StakingBase.sol#L805-L872

    function unstake(uint256 serviceId) external returns (uint256 reward) {
        ServiceInfo storage sInfo = mapServiceInfo[serviceId];
        ...
        // Get the unstaked service data
        reward = sInfo.reward;
        ...
        // Transfer accumulated rewards to the service multisig
        if (reward > 0) {
            _withdraw(multisig, reward);
        }
        ...
    }

https://github.com/code-423n4/2024-05-olas/blob/main/registries/contracts/staking/StakingToken.sol#L104-L111

    function _withdraw(address to, uint256 amount) internal override {
        // Update the contract balance
        balance -= amount;

        SafeTransferLib.safeTransfer(stakingToken, to, amount);
        ...
    }

Proof of Concept

The following steps can occur for the described scenario.

stakingToken is a fee-on-transfer token for the StakingToken contract.
100e18 stakingToken is deposited to the StakingToken contract.
- balance and availableRewards are increased to 100e18.
- The StakingToken contract's balance of stakingToken is 99e18 since a 1% transfer fee of such token was taken.
After staking for a long enough time, a service owner unstakes.
- At this moment, the reward for such service owner has accumulated to equal 100e18.
Because the StakingToken contract only owns 99e18 stakingToken, transferring 100e18 stakingToken from the StakingToken contract to the service owner reverts.
- Hence, the service owner fails to receive the reward of 100e18 stakingToken that is entitled to her or him.

Tools Used

Manual Review

Recommended Mitigation Steps

The deposit function can be updated to increment balance and availableRewards by the value that equals the StakingToken contract's stakingToken balance after the token transfer minus such contract's stakingToken balance before the token transfer.

Assessed type

ERC20

code-423n4 / 2024-05-olas-findings

Service owner can fail to receive reward when `stakingToken` is a fee-on-transfer token #90