GnosisDepositProcessor will never transfer the exact amount of tokens

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-05-olas/blob/3ce502ec8b475885b90668e617f3983cea3ae29f/tokenomics/contracts/staking/GnosisDepositProcessorL1.sol#L63-L70

Vulnerability details

Relevant code: GnosisDepositProcessorL1::_sendMessage

Description

The GnosisDepositProcessorL1 is responsible to bridge token incentives distributed from Dispenser to the Gnosis chain.

The bridging function call consists of the following parameters:

• transferAmount which is the total amount of tokens to transfer (for all target services)
• targets the addresses of the Staking contracts to receive funds
• stakingIncentives an array of uint256 dictating how much each target service should receive

// Approve tokens for the bridge contract
IToken(olas).approve(l1TokenRelayer, transferAmount);

bytes memory data = abi.encode(targets, stakingIncentives);
IBridge(l1TokenRelayer).relayTokensAndCall(olas, l2TargetDispenser, transferAmount, data);

The issue is that the OmniBridge, used to send tokens will take a fee from the amount of tokens sent:

address nativeToken = nativeTokenAddress(_token);
uint256 fee = _distributeFee(HOME_TO_FOREIGN_FEE, nativeToken == address(0), _from, _token, _value);
uint256 valueToBridge = _value.sub(fee);

bytes memory data = _prepareMessage(nativeToken, _token, _receiver, valueToBridge, _data);

This means that when GnosisTargetDispenserL2 receives the message and tries to distribute the token incentives, the last deposit will not occur:

function _processData(bytes memory data) internal {
    ...
    // Decode received data
    (address[] memory targets, uint256[] memory amounts) = abi.decode(data, (address[], uint256[]));
    for (uint256 i = 0; i < targets.length; ++i) {
        address target = targets[i];
        uint256 amount = amounts[i];
        ...
        // Check the OLAS balance and the contract being unpaused
        if (IToken(olas).balanceOf(address(this)) >= amount && localPaused == 1) {
            // Approve and transfer OLAS to the service staking target
            IToken(olas).approve(target, amount);
            IStaking(target).deposit(amount);
            emit StakingTargetDeposited(target, amount);
        } else {
            // Hash of target + amount + batchNonce
            bytes32 queueHash = keccak256(abi.encode(target, amount, batchNonce));
            // Queue the hash for further redeem
            stakingQueueingNonces[queueHash] = true;
            emit StakingRequestQueued(queueHash, target, amount, batchNonce, localPaused);
        }
    }
}

Root Cause

OmniBridge fees are unaccounted for. Contract assumes that all tokens would arrive at the target chain.

Impact

Staking contracts on Gnosis will not be able to automatically receive the token incentives. Every time tokens are bridged, one Staking contract will not get an automatic deposit. This increases the friction of using the protocol and prevents service owners to get their rewards on time.

PoC

Suppose someone calls Dispenser::claimStakingIncentives for a staking target located on the Gnosis chain.

It is calculated that 100 OLAS should be sent to the Staking contract.

When sending the message to GnosisDepositProcessorL1 the following values will be in place:

• transferAmount = 100 OLAS
• targets = [staking contract address] (array of 1 address)
• stakingIncentives = [100e18] - amount to send to staking contract

As the tokens are bridged, a fee will be deducted from transferAmount and it will become 99.5 OLAS. Exact fee amount is without matter.

When the bridging callback is invoked in GnosisTargetDispenserL2 on Gnosis, this balance check will be false (assuming the contract is empty):

if (IToken(olas).balanceOf(address(this)) >= amount && localPaused == 1)

Which means that tokens would not be transferred. Instead, the Staking contract incentive would have to be manually claimed by calling redeem on GnosisTargetDispenserL2.

In case Dispenser::claimStakingIncentivesBatch is called on a bunch of Staking targets, the last one will not be able to receive automatically the deposit.

Suggested Mitigation

A possible solution would be to take into account the token fee when distributing incentives from Dispenser. This means that the IDepositProcessor interface would have to be modified to include a function like function tokenFee(uint256) external returns (uint256);

Then, _distributeStakingIncentives would look like:

...
// Transfer corresponding OLAS amounts to the deposit processor
if (transferAmount > 0) {
    transferAmount += IDepositProcessor(depositProcessor).tokenFee(transferAmount);
    IToken(olas).transfer(depositProcessor, transferAmount);
}
...

Assessed type

Other