search

code-423n4 / 2022-10-thegraph-findings

0 stars 0 forks source link

Compromised Arbitrum: No Sanity/Security Checks on Amount in finalizeInboundTransfer() on Layer 1 #179

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-10-thegraph/blob/309a188f7215fa42c745b136357702400f91b4ff/contracts/gateway/L1GraphTokenGateway.sol#L275-L276

Vulnerability details

Description / Proof of Concept

If L1GraphTokenGateway.finalizeInboundTransfer() receives a valid transaction from the bridge, it will immediately transfer any amount of tokens from escrow.

Impact

This exposes an unnecessarily large attack surface as any compromise of the Arbitrum bridge allows immediate access to the entire balance of the BridgeEscrow contract.

Tools Used

Manual Inspection

Recommended Mitigation Steps

A timelock could be implemented within L1GraphTokenGateway.finalizeInboundTransfer() for any transactions that would release more than a certain percentage, say 10 percent, of the BridgeEscrow contract's token balance. These timelocked token releases would need to be cancellable by a trusted role.

Alternatively, the gateway could pause itself if the bridge's token balance loses more than 10\% within a single day.

These countermeasures would cause little impact to user experience in the majority of calls to the function, but would protect the majority of escrowed funds in the event of a compromise to Arbitrum's bridge.

trust1995 commented 1 year ago

If L2 compromise were to happen, the protocol would have 1 week to pause the L1 contract and prevent L1 from being contaminated.

0xean commented 1 year ago

While I do think suggestions around guarded launches, slowly token releases, are valid for any bridge to consider, I do not believe they qualify as High severity without showing a clear attack. These come down to design choices that all have trade offs with centralization of control and other considerations that have to be weighed carefully.

For example a 10% pause, a whale can DDOS the entire bridge by just transferring their tokens back and forth.... TLDR: QA.

pcarranzav commented 1 year ago

The way I've been thinking about it is users sending GRT through the bridge are trusting Arbitrum enough to deposit their tokens there - which is why our approach is to slowly incentivize the move of the protocol to Arbitrum, rather than enforcing a centralized cap on how much can be moved (which, as @0xean rightly points out, is a DoS vector).