Lines of code

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

Vulnerability details

Impact

The deposit is used to deposit funds for staking. It will transfer tokens to the contract and record the new actual balance and available rewards:

        // 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);

The issue here is if the stakingToken is a fee on transfer token then the actual received amount will be less than input parameter amount and the recorded balance is wrong. For example, A funder deposits 100 tokens to the contract but the actual received amount is 90. However, the balance is recorded to 100. When the staked service tries to withdraw all 100 tokens, there will not be enough tokens.

Impact: The same issue found in other C4 report is defined as Medium

Proof of Concept

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);

        emit Deposit(msg.sender, amount, newBalance, newAvailableRewards);
    }

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

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

        SafeTransferLib.safeTransfer(stakingToken, to, amount);

        emit Withdraw(to, amount);
    }

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

Tools Used

Manual Review

Recommended Mitigation Steps

It is recommended to record actual received amount:

function deposit(uint256 amount) external {
        uint256 before = IERC20(stakingToken).balanceOf(address(this));
        // Add to the overall balance
        SafeTransferLib.safeTransferFrom(stakingToken, msg.sender, address(this), amount);
        uint256 amount = IERC20(stakingToken).balanceOf(address(this)) - before;
        // 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;
        emit Deposit(msg.sender, amount, newBalance, newAvailableRewards);
    }

Assessed type

ERC20

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

If the underlying asset is a fee on transfer token, the `_withdraw` may fail #102