fix: address tmnt 150

Author: LHerskind

l1-contracts/gas_benchmark.md

@@ -29,7 +29,7 @@
 |----------------------|---------|---------|---------------|--------------|
 | propose              | 206,977 | 226,338 |         2,852 |       45,632 |
 | submitEpochRootProof | 679,690 | 701,700 |         5,092 |       81,472 |
-| aggregate3           | 256,679 | 286,474 |             - |            - |
+| aggregate3           | 256,810 | 286,606 |             - |            - |
 | setupEpoch           |  38,145 | 327,074 |             - |            - |
 
 **Avg Gas Cost per Second**: 1,229.6 gas/second
@@ -65,7 +65,7 @@
 |----------------------|---------|---------|---------------|--------------|
 | propose              | 334,025 | 351,385 |         4,580 |       73,280 |
 | submitEpochRootProof | 895,025 | 933,081 |         6,308 |      100,928 |
-| aggregate3           | 386,141 | 411,884 |             - |            - |
+| aggregate3           | 386,274 | 412,016 |             - |            - |
 | setupEpoch           |  49,728 | 542,190 |             - |            - |
 
 **Avg Gas Cost per Second**: 10,875.5 gas/second

l1-contracts/gas_benchmark_results.json

@@ -55,10 +55,10 @@
       },
       "aggregate3": {
         "calls": 100,
-        "min": 243742,
-        "mean": 256679,
-        "median": 253598,
-        "max": 286474
+        "min": 243874,
+        "mean": 256810,
+        "median": 253736,
+        "max": 286606
       }
     }
   },
@@ -118,10 +118,10 @@
       },
       "aggregate3": {
         "calls": 100,
-        "min": 362550,
-        "mean": 386141,
-        "median": 384626,
-        "max": 411884
+        "min": 362682,
+        "mean": 386274,
+        "median": 384758,
+        "max": 412016
       }
     }
   }

l1-contracts/src/governance/proposer/EmpireBase.sol

@@ -292,6 +292,7 @@ abstract contract EmpireBase is EIP712, IEmpire {
     require(
       currentSlot > round.lastSignalSlot.decompress(), Errors.GovernanceProposer__SignalAlreadyCastForSlot(currentSlot)
     );
+    round.lastSignalSlot = currentSlot.compress();
 
     address signaler = selection.getCurrentProposer();
 
@@ -306,7 +307,6 @@ abstract contract EmpireBase is EIP712, IEmpire {
     }
 
     round.signalCount[_payload] += 1;
-    round.lastSignalSlot = currentSlot.compress();
 
     // @todo We can optimise here for gas by storing some of it packed with the payloadWithMostSignals.
     if (

l1-contracts/test/governance/governance-proposer/mocks/Fakerollup.sol

@@ -47,7 +47,7 @@ contract Fakerollup {
     return TimeLib.getStorage().proofSubmissionEpochs;
   }
 
-  function getCurrentProposer() external view returns (address) {
+  function getCurrentProposer() external virtual returns (address) {
     return proposer;
   }
 

l1-contracts/test/governance/governance-proposer/scenario/tmnt150.t.sol

@@ -0,0 +1,97 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity >=0.8.27;
+
+import {GovernanceProposerBase} from "../Base.t.sol";
+import {GovernanceProposer} from "@aztec/governance/proposer/GovernanceProposer.sol";
+
+import {IPayload} from "@aztec/governance/interfaces/IPayload.sol";
+import {Slot, Timestamp} from "@aztec/core/libraries/TimeLib.sol";
+import {Fakerollup} from "../mocks/Fakerollup.sol";
+import {IRollup} from "@aztec/core/interfaces/IRollup.sol";
+import {Signature} from "@aztec/shared/libraries/SignatureLib.sol";
+import {MessageHashUtils} from "@oz/utils/cryptography/MessageHashUtils.sol";
+import {Errors} from "@aztec/governance/libraries/Errors.sol";
+
+contract MaliciousRollup is Fakerollup {
+  uint256 public numberOfRounds;
+  GovernanceProposer public governanceProposer;
+  IPayload public proposal;
+
+  function maliciousValues(uint256 _numberOfRounds, GovernanceProposer _governanceProposer, IPayload _proposal)
+    external
+  {
+    numberOfRounds = _numberOfRounds;
+    governanceProposer = _governanceProposer;
+    proposal = _proposal;
+  }
+
+  function commenceAttack() external {
+    governanceProposer.signal(proposal);
+  }
+
+  function getCurrentProposer() external override returns (address) {
+    if (numberOfRounds <= 1) {
+      return address(this);
+    }
+
+    numberOfRounds--;
+
+    governanceProposer.signal(proposal);
+
+    return address(this);
+  }
+}
+
+// https://linear.app/aztec-labs/issue/TMNT-150/spearbit-gov-finding-2-potential-reentrancy-with-malicious-rollup
+contract TestTmnt150 is GovernanceProposerBase {
+  using MessageHashUtils for bytes32;
+
+  IPayload internal proposal = IPayload(address(0xdeadbeef));
+  address internal proposer = address(0);
+  MaliciousRollup internal rollup1;
+
+  uint256 internal privateKey = 0x1234567890;
+  Signature internal signature;
+
+  function setUp() public override {
+    super.setUp();
+  }
+
+  function test_reentry() external {
+    rollup1 = new MaliciousRollup();
+
+    proposer = vm.addr(privateKey);
+    rollup1.setProposer(proposer);
+
+    vm.prank(registry.getGovernance());
+    registry.addRollup(IRollup(address(rollup1)));
+    vm.warp(Timestamp.unwrap(rollup1.getTimestampForSlot(Slot.wrap(1))));
+
+    // Create a signature for the proposer
+    uint256 round = governanceProposer.getCurrentRound();
+    signature = createSignature(privateKey, address(proposal), round);
+
+    rollup1.maliciousValues(5, governanceProposer, proposal);
+
+    assertEq(governanceProposer.signalCount(address(rollup1), 0, proposal), 0, "invalid number of votes");
+
+    vm.expectRevert(abi.encodeWithSelector(Errors.GovernanceProposer__SignalAlreadyCastForSlot.selector, 1));
+
+    rollup1.commenceAttack();
+
+    assertEq(governanceProposer.signalCount(address(rollup1), 0, proposal), 0, "invalid number of votes");
+  }
+
+  function createSignature(uint256 _privateKey, address _payload, uint256 _round)
+    internal
+    view
+    returns (Signature memory)
+  {
+    address signer = vm.addr(_privateKey);
+    bytes32 digest = governanceProposer.getSignalSignatureDigest(IPayload(_payload), signer, _round);
+
+    (uint8 v, bytes32 r, bytes32 s) = vm.sign(_privateKey, digest);
+
+    return Signature({v: v, r: r, s: s});
+  }
+}