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

0 stars 0 forks source link

Pause functionality locks funds if applied to only one gateway #213

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#L198 https://github.com/code-423n4/2022-10-thegraph/blob/309a188f7215fa42c745b136357702400f91b4ff/contracts/gateway/L1GraphTokenGateway.sol#L269

Vulnerability details

The Pause guardian is a separate entity from the governor that can pause the gateway contract.

Both transfers (deposit L1 to L2, and withdraw from L2 back to L1) involve one function on the L1Gateway and one function on the L2Gateway.

L1GraphTokenGateway.outboundTransfer()
L1GraphTokenGateway.finalizeInboundTransfer()
L2GraphTokenGateway.outboundTransfer()
L2GraphTokenGateway.finalizeInboundTransfer()

These four functions have a notPaused modifier and cannot be called if the contract is paused.

The problem is that pausing is not synchronous across the gateways: pausing L1GraphTokenGateway does not trigger pausing in L2GraphTokenGateway, and vice-versa.

This means it is possible for a pauseGuardian (which is a separate entity from the governor) to pause one of the gateways while withdrawals are pending, effectively freezing the users GRT on L1.

The issue arises mainly because pausing the gateways is instantaneous (which is understandable for emergency reasons) while the Arbitrum dispute period is quite lengthy (~ 1 week)

Impact

Medium

Proof Of Concept

Tools Used

Manual Analysis

Mitigation

You can ensure the pausing mechanism happen in both L1GraphTokenGateway and L2GraphTokenGateway instead of being independent from each other. For instance:

0xean commented 1 year ago

dupe of #195