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

0 stars 0 forks source link

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

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-10-thegraph/blob/309a188f7215fa42c745b136357702400f91b4ff/contracts/l2/gateway/L2GraphTokenGateway.sol#L236

Vulnerability details

Description / Proof of Concept

If L2GraphTokenGateway.finalizeInboundTransfer() receives a valid transaction from the bridge, it will immediately mint any amount of GRT tokens on Arbitrum.

Impact

This exposes an unnecessarily large attack surface as any compromise of the Arbitrum bridge allows immediate minting to unlimited numbers of L2 GRT tokens.

Tools Used

Manual Inspection

Recommended Mitigation Steps

Unlike the similar issue with L1GraphTokenGateway.finalizeInboundTransfer(), mitigation of this issue would require an admin controlled daily mint limit within the L2GraphToken contract, or the L2GraphTokenGateway.finalizeInboundTransfer() function.

For any transactions that would exceed this limit, the mint request could either be timelocked, or it could revert and pause the protocol. These timelocked mints would need to be cancellable by a trusted role.

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.

0xean commented 2 years ago

dupe of #179