Fungibility of `GRT` can be affected by `burn()`

[code423n4]

Lines of code

https://github.com/code-423n4/2022-10-thegraph/blob/309a188f7215fa42c745b136357702400f91b4ff/contracts/l2/token/L2GraphToken.sol#L15

Vulnerability details

GIP-0031 describes the importance to keep GRT on L1 and L2 fungible:

Keeping the tokens locked ensures the GRT on both sides are fungible, as the same amount that’s locked is the total amount that exists on L2.

This contradicts what is also said in the same document: that Burning can be done as usual, ie by calling ERC20BurnableUpgradeable.burn()

For now, tokens burned using burn() will not trigger any special behavior, but future iterations may sync back to L1 with a special call to burn tokens on the L1 side of the bridge or handle it some other way.

This means that users calling ERC20BurnableUpgradeable.burn() on L2 will result in the same amount of tokens frozen in the bridge escrow on L1: the total supplies of GRT will be different on L1 and L2.

Impact

Medium

Tools Used

Manual Analysis

Mitigation

You can choose to update L2GraphToken.burn() and L2GraphToken.burnFrom() so that they call L2ArbitrumMessenger.sendTxToL1() to send a call to burn tokens on L1. Note that for L2GraphToken.burnFrom(), you will need to ensure that a call is not made if the call comes from L2GraphToken.bridgeBurn(), as that function already performs a call to L1.

Or a simpler solution is to prevent users from burning their GRT on Arbitrum.

in L2GraphToken.sol add:

+function burn(uint256 amount) public override(ERC20BurnableUpgradeable) {
+    revert();
+}

+function burnFrom(address account, uint256 amount) public override(ERC20BurnableUpgradeable) {
+    require(_msgSender() = address(this)); //@audit this ensures it can only be called via bridgeBurn()
+    _spendAllowance(account, _msgSender(), amount);
+    _burn(account, amount);
+}

[0xean]

downgrading to QA.

[pcarranzav]

This is actually mentioned in the risk register 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