Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Acknowledgement processing can be return bombed due to use of address.send leading to loss of funds #70

Open
hats-bug-reporter bot opened this issue Jan 31, 2024 · 1 comment
Labels
invalid This doesn't seem right

Comments

@hats-bug-reporter
Copy link

Github username: --
Twitter username: --
Submission hash (on-chain): 0xf02c0b7844e9d7a3abbeb774c75a730330692d61cd56b91c1ada9f7f49027e30
Severity: high

Description:
Description
The IncentivizedMessageEscrow._handleAck function performs Solidity's send call to transfer native tokens.

        if(!payable(refundGasTo).send(refund)) {
            payable(SEND_LOST_GAS_TO).transfer(refund); 
        }

https://github.com/catalystdao/GeneralisedIncentives/blob/main/src/IncentivizedMessageEscrow.sol#L437-L439

Even though the send call only forwards 2300 gas still it is susceptible to return-data-bomb attack, i.e, the recipient can return a huge amount of data which will be copied to memory of _handleAck call. This happens by default. This large amount of returned data can cause the _handleAck call to revert due to block gas limit.

Hence the acknowledgement cannot be processed, all attempts of calling IncentivizedMessageEscrow.processPacket will get reverted.

This can lead to huge amount of losses for protocol.

Attack Scenario

  1. Attacker calls Vault.sendAssets with refundGasTo set to a malicious contract address. The tokens deposited by attacker gets added to escrowed amount in vault.
  2. Relayers submit IncentivizedMessageEscrow.processPacket call on destination chain. Attacker receives funds on destination chain.
  3. Relayers call IncentivizedMessageEscrow.processPacket to submit acknowledgement on source chain, the function tries to call send on malicious refundGasTo address.
  4. The refundGasTo performs the return data bomb and the processPacket (ack) call gets reverted.
  5. Attacker's deposited amount remains in the escrowed accounting, processPacket cannot be executed. Further recoverAck cannot be executed due to this statement.

The escrowed tokens are lost forever.

The attack can be performed by any of the recipients of the send call.

Attachments

  1. Proof of Concept (PoC) File
    Provided above

  2. Revised Code File (Optional)
    Consider using assembly for transferring funds to refundGasTo address.

@hats-bug-reporter hats-bug-reporter bot added the bug Something isn't working label Jan 31, 2024
@reednaa
Copy link
Collaborator

reednaa commented Jan 31, 2024

Dude, solidity...

We sadly need a PoC for this. You can use our return bomber contact to create a PoC:
https://github.com/catalystdao/GeneralisedIncentives/blob/main/test/mocks/ReturnBomber.sol

The reason why I need a PoC is because I don't think it is that bad. You can't do a lot of damage in 2300 gas.

@reednaa reednaa added the question Further information is requested label Jan 31, 2024
@reednaa reednaa added invalid This doesn't seem right and removed bug Something isn't working question Further information is requested labels Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

1 participant