search

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

0 stars 0 forks source link

GRT is inflated by escrowing L1 tokens instead of burning them #299

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/l2/gateway/L2GraphTokenGateway.sol#L1-L298 https://github.com/code-423n4/2022-10-thegraph/blob/309a188f7215fa42c745b136357702400f91b4ff/contracts/gateway/L1GraphTokenGateway.sol#L1-L360 https://github.com/code-423n4/2022-10-thegraph/blob/309a188f7215fa42c745b136357702400f91b4ff/contracts/gateway/BridgeEscrow.sol#L28-L30 https://github.com/code-423n4/2022-10-thegraph/blob/309a188f7215fa42c745b136357702400f91b4ff/contracts/governance/Governed.sol#L1-L69

Vulnerability details

Impact

GRT is inflated by being both in L2 and the escrow.

Proof of Concept

When tokens are sent from L2 to L1 they are simply burnt in L2 and released in L1 from previously escrowed tokens. When tokens are sent from L1 to L2, however, they are escrowed instead of burnt. This together with the fact that the governor can approve spending of the escrowed tokens to an arbitrary address, and that the governer cannot be renounced, means that the tokens on L1 are still able to be made available to the market, effectively having doubled the supply of GRT at the end of the migration.

Tools Used

Code inspection

Recommended Mitigation Steps

Burn L1 tokens which exist on L2.

0xean commented 1 year ago

Downgrading to QA, this ultimately is a design choice and the wardens reasoning behind the tokens are available to the market is not clear.

0xean commented 1 year ago

dupe of #302 - wardens QA

pcarranzav commented 1 year ago

This is also a known risk described in GIP-0031, see the last table entry in https://forum.thegraph.com/t/gip-0031-arbitrum-grt-bridge/3305#risks-and-security-considerations-15