KingNFT - Governance manipulation attack on ````NativeTokens````

[sherlock-admin3]

KingNFT

High

Governance manipulation attack on NativeTokens

Summary

With the help of governance platforms, such as snapshot.org and aragon.org, thousands of standard ERC20 tokens are working as governance tokens. If these tokens were set as NativeTokens, the current improper implementation of OptimismPortal2 would cause those tokens suffering governance manipulation attack.

Root Cause

(1) In high probability, there would be large amount of NativeToken held by OptimismPortal2

As reference from Optimism and Base, they both have about 1.3 Billion USD worth of NativeToken(ETH in their case) held in OptimismPortal, we can expect large amount of NativeToken would be held by OptimismPortal2 too, as OptimismPortal2 will be the L1 vault for NativeTokens which are being deposited to L2.

(2) For messages sent from L2 to L1, the target can be any contract except NativeToken itself

Shown as L382-384 of OptimismPortal2.sol

File: src\L1\OptimismPortal2.sol
356:     function finalizeWithdrawalTransactionExternalProof(
357:         Types.WithdrawalTransaction memory _tx,
358:         address _proofSubmitter
359:     )
...
362:     {
...
382:         require(
383:             _tx.target != _nativeTokenAddress, "Optimism Portal: cannot make a direct call to native token contract"
384:         );
...
429:     }

Internal pre-conditions

Any of these governance token are set as NativeToken

External pre-conditions

N/A

Attack Path

(1) Attack case for governance tokens using snapshot.org platform

We can find there are thousands of projects are using ERC20 balance as voting power (https://snapshot.org/#/?filter=strategies&q=erc20-balance-of), and many of them enabled voting delegation. If any of these tokens are also working as NativeToken, then attackers can send a L2 -> L1 message with snapshot's unified DelegateRegistry contract (link) as target to delegate OptimismPortal2 's huge voting power to some malicious address controlled by attacker.

File: https://etherscan.io/address/0x469788fE6E9E9681C6ebF3bF78e7Fd26Fc015446#code:DelegateRegistry.sol
19:     function setDelegate(bytes32 id, address delegate) public {
 // @audit msg.sender=OptimismPortal2, id= governance space, delegate= malicious EOA
20:         require (delegate != msg.sender, "Can't delegate to self");
21:         require (delegate != address(0), "Can't delegate to 0x0");
22:         address currentDelegate = delegation[msg.sender][id];
23:         require (delegate != currentDelegate, "Already delegated to this address");
24:         
25:         // Update delegation mapping
26:         delegation[msg.sender][id] = delegate;
27:         
28:         if (currentDelegate != address(0)) {
29:             emit ClearDelegate(msg.sender, id, currentDelegate);
30:         }
31: 
32:         emit SetDelegate(msg.sender, id, delegate);
33:     }

(2) Attack case for governance tokens using aragon.org framework

Under argon framework, there are separated voting contract instances for each project, let's take the well known Lido project for example https://vote.lido.fi/, the governance token is LDO:0x5A98FcBEA516Cf06857215779Fd812CA3beF1B32 (link: CoinMarketCap), and it's argon voting contract is 0x2e59A20f205bB85a89C53f1936454680651E618e. If LDO was working as NativeToken, then attackers can send a L2 -> L1 message with the argon voting contract as target to delegate OptimismPortal2 's huge voting power to some malicious address by calling assignDelegate()

File: https://etherscan.io/address/0x2e59a20f205bb85a89c53f1936454680651e618e#Voting.sol
259:     function assignDelegate(address _delegate) external { // @audit __delegate=malicious account
260:         require(_delegate != address(0), ERROR_ZERO_ADDRESS_PASSED);
261:         require(_delegate != msg.sender, ERROR_SELF_DELEGATE);
262: 
263:         address prevDelegate = delegates[msg.sender].delegate; // @audit msg.sender=OptimismPortal2
264:         require(_delegate != prevDelegate, ERROR_DELEGATE_SAME_AS_PREV);
265: 
266:         if (prevDelegate != address(0)) {
267:             _removeDelegatedAddressFor(prevDelegate, msg.sender);
268:         }
269:         _addDelegatedAddressFor(_delegate, msg.sender);
270:     }

Or, calling vote() to directly vote for some malicious proposal

File: https://etherscan.io/address/0x2e59a20f205bb85a89c53f1936454680651e618e#Voting.sol
232:     function vote(uint256 _voteId, bool _supports, bool /* _executesIfDecided_deprecated */) external voteExists(_voteId) {
233:         Vote storage vote_ = votes[_voteId];
234:         VotePhase votePhase = _getVotePhase(vote_);
235:         require(_isValidPhaseToVote(votePhase, _supports), ERROR_CAN_NOT_VOTE);
236: 
...
240:         uint256 votingPower = token.balanceOfAt(msg.sender, vote_.snapshotBlock);
241:         require(votingPower > 0, ERROR_NO_VOTING_POWER);
242:         _vote(_voteId, votePhase, /* voter */ msg.sender, _supports, votingPower, /* isDelegate */ false);
243:     }

(3) Though this report doesn't check all governance platforms and frameworks, but there are high chance the other governance frameworks and tokens suffer similar attack vectors

Impact

Widely governance manipulation attack to produce unconstrained damage

PoC

No response

Mitigation

Adding a target blacklist to allow the admin to manage dangerous targets of all kinds, such as

diff --git a/tokamak-thanos/packages/tokamak/contracts-bedrock/src/L1/OptimismPortal2.sol b/tokamak-thanos/packages/tokamak/contracts-bedrock/src/L1/OptimismPortal2.sol
index 8c5af46..679609c 100644
--- a/tokamak-thanos/packages/tokamak/contracts-bedrock/src/L1/OptimismPortal2.sol
+++ b/tokamak-thanos/packages/tokamak/contracts-bedrock/src/L1/OptimismPortal2.sol
@@ -104,6 +104,8 @@ contract OptimismPortal2 is Initializable, ResourceMetering, OnApprove, ISemver
     /// @notice Spacer for forwards compatibility.
     bytes32 private spacer_61_0_32;

+    mapping(address => bool) targetBlackList;
+
     /// @notice Emitted when a transaction is deposited from L1 to L2.
     ///         The parameters of this event are read by the rollup node and used to derive deposit
     ///         transactions on L2.
@@ -383,6 +385,9 @@ contract OptimismPortal2 is Initializable, ResourceMetering, OnApprove, ISemver
             _tx.target != _nativeTokenAddress, "Optimism Portal: cannot make a direct call to native token contract"
         );

+        require(
+            !targetBlackList[_tx.target], "Optimism Portal: cannot make a direct call to blacklist contract"        
+        );
         // Set the l2Sender so contracts know who triggered this withdrawal on L2.
         l2Sender = _tx.sender;