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

13 stars 4 forks source link

Attacker can make claimed staking incentives irredeemable on Gnosis Chain #33

Open c4-bot-4 opened 5 months ago

c4-bot-4 commented 5 months ago

Lines of code

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

Vulnerability details

The GnosisDepositProcessorL1 contract is responsible for bridging OLAS tokens and messages from L1 to the GnosisTargetDispenserL2 contract on Gnosis Chain. When calling claimStakingIncentives() or claimStakingIncentivesBatch() on the Dispenser contract for a nominee on Gnosis, it calls _sendMessage() internally to send the incentive amounts and any necessary funds to L2.

If the transferAmount has been reduced to 0 due to sufficient withheld funds on Gnosis, it only sends a message via the Arbitrary Message Bridge (AMB) to process the claimed incentives.

However, when sending this message, the caller provides a gasLimitMessage parameter to specify the gas limit for executing the message on L2. There is no lower bound enforced on this value, although it is required to be >= 100 internally by the AMB. If the caller sets gasLimitMessage too low, the message will fail to execute on L2 due to out of gas.

Because the Gnosis AMB provides no native way to replay failed messages, if a message fails due to insufficient gas, the claimed staking incentives will be irredeemable on L2. An attacker could intentionally grief the protocol by repeatedly claiming incentives with an insufficient gas limit, causing them to be lost.

While the protocol could call processDataMaintenance() on L2 with the data from the cancelled message to recover, this would require passing a governance proposal. Given that this attack can be carried out any number of times (e.g. as often as possible and with a different target each time), this can considerably impact the functioning of the protocol hence I believe Medium severity is justified.

Impact

An attacker can make legitimately claimed staking incentives irredeemable on L2 by setting an insufficient gas limit when claiming. This would require governance intervention each time to recover the lost incentives.

Proof of Concept

  1. Some amount of OLAS is withheld in the GnosisTargetDispenserL2 contract
  2. The amount is synced to the Dispenser on L1 via GnosisTargetDispenserL2.syncWithheldtokens()
  3. Attacker calls claimStakingIncentives() on the Dispenser contract for a nominee on Gnosis Chain that has accumulated less rewards than the withheld amount, providing an insufficient gasLimitMessage in the bridgePayload
  4. Message is sent via AMB but fails to execute on L2 due to out of gas
  5. Claimed staking incentives are now stuck and irredeemable on L2

Tools Used

Manual review

Recommended Mitigation Steps

Do not allow users to directly specify the gasLimitMessage. Instead, have the GnosisDepositProcessorL1 contract calculate the required gas based on the number of targets and stakingIncentives being processed. A small buffer can be added to account for any minor differences in gas costs.

Alternatively, implement a way to replay failed messages from the AMB, so that irredeemable incentives can be recovered without requiring governance intervention.

Assessed type

Other

c4-judge commented 5 months ago

0xA5DF marked the issue as not a duplicate

c4-judge commented 5 months ago

0xA5DF changed the severity to QA (Quality Assurance)

c4-judge commented 5 months ago

This previously downgraded issue has been upgraded by 0xA5DF

c4-judge commented 5 months ago

This previously downgraded issue has been upgraded by 0xA5DF

c4-sponsor commented 5 months ago

kupermind (sponsor) acknowledged

c4-judge commented 5 months ago

0xA5DF marked the issue as satisfactory

c4-judge commented 5 months ago

0xA5DF marked the issue as selected for report

vsharma4394 commented 4 months ago

Fixed

thebrittfactor commented 3 months ago

Olas sponsor (kupermind) confirmed via outside Github:

The issue has been fixed due to the code re-writing and making the gas limit much higher by default compared to the warden's concern. Even though there's no specific PR, the issue is fixed in between all the relevant PRs.