jkilpatr
commented
2 years ago As mentioned in #58 gas estimation is a handled error here. There may be some confusion where the reporter is expecting the error to occur when the payload is encoded, but that will not occur as an invalid transaction is a perfectly valid abi encoded payload.
Now the gas estimation in this case will fail, but it will not disrupt any other relayer operations while doing so.
We intentionally chose to store the batch nonce before the transfers so that a failure half way through would not create a double spend situation (place a send that fails half way through a batch, then execute the same batch again and again).
I don't see how this is a valid bug, it won't block relaying, users will lose tokens yes, but that's inevitable after the blacklisting proposed, there's no possible mitigation on our side that will let those users get their tokens back on Ethereum.
albertchon
loudoguno
Handle
hack3r-0m
Vulnerability details
https://github.com/althea-net/cosmos-gravity-bridge/blob/main/solidity/contracts/Gravity.sol#L400
At the above-mentioned places in Gravity contract, it makes external call to a function to transfer erc20 token. This can cause revert in cases where erc20 safeTransfer fails (for e.g erc20 contract has blacklisted address of gravity contract to alllow transfers) and hence,
TransactionBatchExecutedEventevent will not be emitted resulting in pending state and not updating nonces.https://github.com/althea-net/cosmos-gravity-bridge/blob/92d0e12cea813305e6472851beeb80bd2eaf858d/orchestrator/relayer/src/batch_relaying.rs#L229-L244
At relayer level, gas estimation will fail and result in panic while unwrapping. If such transfer transactions puts high fee to get picked up by relayer then especially causing more damage.
Introduce a mechanism to filter such scenarios so they are not picked by relayers frequently