fix: address tmnt 141 (#16447)

Please read [contributing guidelines](CONTRIBUTING.md) and remove this
line.

For audit-related pull requests, please use the [audit PR
template](?expand=1&template=audit.md).

Author: LHerskind

l1-contracts/gas_benchmark.md

@@ -27,12 +27,12 @@
 
 | Function             | Avg Gas | Max Gas | Calldata Size | Calldata Gas |
 |----------------------|---------|---------|---------------|--------------|
-| propose              | 206,977 | 226,338 |         2,852 |       45,632 |
+| propose              | 207,888 | 227,249 |         2,852 |       45,632 |
 | submitEpochRootProof | 679,690 | 701,700 |         5,092 |       81,472 |
-| aggregate3           | 244,745 | 264,110 |             - |            - |
+| aggregate3           | 245,657 | 265,021 |             - |            - |
 | setupEpoch           |  38,145 | 327,074 |             - |            - |
 
-**Avg Gas Cost per Second**: 1,229.6 gas/second
+**Avg Gas Cost per Second**: 1,234.4 gas/second
 *Epoch duration*: 2h 33m 36s
 
 
@@ -63,11 +63,11 @@
 
 | Function             | Avg Gas | Max Gas | Calldata Size | Calldata Gas |
 |----------------------|---------|---------|---------------|--------------|
-| propose              | 334,025 | 351,385 |         4,580 |       73,280 |
+| propose              | 334,960 | 352,320 |         4,580 |       73,280 |
 | submitEpochRootProof | 895,025 | 933,081 |         6,308 |      100,928 |
-| aggregate3           | 372,157 | 389,520 |             - |            - |
+| aggregate3           | 373,091 | 390,443 |             - |            - |
 | setupEpoch           |  49,728 | 542,190 |             - |            - |
 
-**Avg Gas Cost per Second**: 10,875.5 gas/second
+**Avg Gas Cost per Second**: 10,901.5 gas/second
 *Epoch duration*: 0h 19m 12s
 

l1-contracts/gas_benchmark_results.json

@@ -30,10 +30,10 @@
     "validators": {
       "propose": {
         "calls": 100,
-        "min": 200706,
-        "mean": 206977,
-        "median": 207326,
-        "max": 226338,
+        "min": 201617,
+        "mean": 207888,
+        "median": 208237,
+        "max": 227249,
         "calldata_size": 2852,
         "calldata_gas": 45632
       },
@@ -55,10 +55,10 @@
       },
       "aggregate3": {
         "calls": 100,
-        "min": 238466,
-        "mean": 244745,
-        "median": 245098,
-        "max": 264110
+        "min": 239389,
+        "mean": 245657,
+        "median": 245991,
+        "max": 265021
       }
     }
   },
@@ -93,10 +93,10 @@
     "validators": {
       "propose": {
         "calls": 100,
-        "min": 318448,
-        "mean": 334025,
-        "median": 334471,
-        "max": 351385,
+        "min": 319383,
+        "mean": 334960,
+        "median": 335406,
+        "max": 352320,
         "calldata_size": 4580,
         "calldata_gas": 73280
       },
@@ -118,10 +118,10 @@
       },
       "aggregate3": {
         "calls": 100,
-        "min": 356583,
-        "mean": 372157,
-        "median": 372603,
-        "max": 389520
+        "min": 357506,
+        "mean": 373091,
+        "median": 373535,
+        "max": 390443
       }
     }
   }

l1-contracts/src/core/libraries/Errors.sol

@@ -162,6 +162,12 @@ library Errors {
   // SignatureLib (duplicated)
   error SignatureLib__InvalidSignature(address, address); // 0xd9cbae6c
 
+  error AttestationLib__OutOfBounds(uint256, uint256);
+  error AttestationLib__SignatureIndicesSizeMismatch(uint256, uint256);
+  error AttestationLib__SignaturesOrAddressesSizeMismatch(uint256, uint256);
+  error AttestationLib__NotASignatureAtIndex(uint256 index);
+  error AttestationLib__NotAnAddressAtIndex(uint256 index);
+
   // RewardBooster
   error RewardBooster__OnlyRollup(address caller);
 }

l1-contracts/src/core/libraries/rollup/AttestationLib.sol

@@ -2,6 +2,7 @@
 // Copyright 2024 Aztec Labs.
 pragma solidity ^0.8.27;
 
+import {Errors} from "@aztec/core/libraries/Errors.sol";
 import {Signature, SignatureLib} from "@aztec/shared/libraries/SignatureLib.sol";
 
 /**
@@ -71,7 +72,7 @@ library AttestationLib {
     returns (Signature memory)
   {
     bytes memory signaturesOrAddresses = _attestations.signaturesOrAddresses;
-    require(isSignature(_attestations, _index), "Not a signature at this index");
+    require(isSignature(_attestations, _index), Errors.AttestationLib__NotASignatureAtIndex(_index));
 
     uint256 dataPtr;
     assembly {
@@ -105,7 +106,7 @@ library AttestationLib {
    */
   function getAddress(CommitteeAttestations memory _attestations, uint256 _index) internal pure returns (address) {
     bytes memory signaturesOrAddresses = _attestations.signaturesOrAddresses;
-    require(!isSignature(_attestations, _index), "A signature at this index");
+    require(!isSignature(_attestations, _index), Errors.AttestationLib__NotAnAddressAtIndex(_index));
 
     uint256 dataPtr;
     assembly {
@@ -126,9 +127,56 @@ library AttestationLib {
     return addr;
   }
 
+  /**
+   * @notice Assert that the size of `_attestations` is as expected, throw otherwise
+   *
+   * @custom:reverts SignatureIndicesSizeMismatch if the signature indices have a wrong size
+   * @custom:reverts SignaturesOrAddressesSizeMismatch if the signatures or addresses object has wrong size
+   *
+   * @param _attestations - The attestation struct
+   * @param _expectedCount - The expected size of the validator set
+   */
+  function assertSizes(CommitteeAttestations memory _attestations, uint256 _expectedCount) internal pure {
+    // Count signatures (1s) and addresses (0s) from bitmap
+    uint256 signatureCount = 0;
+    uint256 addressCount = 0;
+    uint256 bitmapBytes = (_expectedCount + 7) / 8; // Round up to nearest byte
+    require(
+      bitmapBytes == _attestations.signatureIndices.length,
+      Errors.AttestationLib__SignatureIndicesSizeMismatch(bitmapBytes, _attestations.signatureIndices.length)
+    );
+
+    for (uint256 i = 0; i < _expectedCount; i++) {
+      uint256 byteIndex = i / 8;
+      uint256 bitIndex = 7 - (i % 8);
+      uint8 bitMask = uint8(1 << bitIndex);
+
+      if (uint8(_attestations.signatureIndices[byteIndex]) & bitMask != 0) {
+        signatureCount++;
+      } else {
+        addressCount++;
+      }
+    }
+
+    // Calculate expected size
+    uint256 sizeOfSignaturesAndAddresses = (signatureCount * SIGNATURE_LENGTH) + (addressCount * ADDRESS_LENGTH);
+
+    // Validate actual size matches expected
+    require(
+      sizeOfSignaturesAndAddresses == _attestations.signaturesOrAddresses.length,
+      Errors.AttestationLib__SignaturesOrAddressesSizeMismatch(
+        sizeOfSignaturesAndAddresses, _attestations.signaturesOrAddresses.length
+      )
+    );
+  }
+
   /**
    * Recovers the committee from the addresses in the attestations and signers.
    *
+   * @custom:reverts SignatureIndicesSizeMismatch if the signature indices have a wrong size
+   * @custom:reverts OutOfBounds throws if reading data beyond the `_attestations`
+   * @custom:reverts SignaturesOrAddressesSizeMismatch if the signatures or addresses object has wrong size
+   *
    * @param _attestations - The committee attestations
    * @param _signers The addresses of the committee members that signed the attestations. Provided in order to not have
    * to recover them from their attestations' signatures (and hence save gas). The addresses of the non-signing
@@ -141,8 +189,14 @@ library AttestationLib {
     address[] memory _signers,
     uint256 _length
   ) internal pure returns (address[] memory) {
+    uint256 bitmapBytes = (_length + 7) / 8; // Round up to nearest byte
+    require(
+      bitmapBytes == _attestations.signatureIndices.length,
+      Errors.AttestationLib__SignatureIndicesSizeMismatch(bitmapBytes, _attestations.signatureIndices.length)
+    );
+
+    // To get a ref that we can easily use with the assembly down below.
     bytes memory signaturesOrAddresses = _attestations.signaturesOrAddresses;
-    bytes memory signatureIndices = _attestations.signatureIndices;
     address[] memory addresses = new address[](_length);
 
     uint256 signersIndex;
@@ -154,12 +208,13 @@ library AttestationLib {
       // Skip length
       dataPtr := add(signaturesOrAddresses, 0x20)
     }
+    uint256 offset = dataPtr;
 
     for (uint256 i = 0; i < _length; ++i) {
       // Load new byte every 8 iterations
       if (i % 8 == 0) {
         uint256 byteIndex = i / 8;
-        currentByte = uint8(signatureIndices[byteIndex]);
+        currentByte = uint8(_attestations.signatureIndices[byteIndex]);
         bitMask = 128; // 0b10000000
       }
 
@@ -180,6 +235,23 @@ library AttestationLib {
       }
     }
 
+    // Ensure that the reads were within the boundaries of the data.
+    // As `dataPtr` will always be increasing (and unlikely to wrap around because it would require insane size)
+    // we can just check that the last dataPtr value is inside the limit, as all the others would be as well then.
+    uint256 upperLimit = offset + _attestations.signaturesOrAddresses.length;
+    // As the offset was added already part of both values, we can subtract to give a more meaningful error.
+    require(dataPtr <= upperLimit, Errors.AttestationLib__OutOfBounds(dataPtr - offset, upperLimit - offset));
+
+    // Ensure that the size of data provided actually matches what we expect
+    uint256 sizeOfSignaturesAndAddresses =
+      (signersIndex * SIGNATURE_LENGTH) + ((_length - signersIndex) * ADDRESS_LENGTH);
+    require(
+      sizeOfSignaturesAndAddresses == _attestations.signaturesOrAddresses.length,
+      Errors.AttestationLib__SignaturesOrAddressesSizeMismatch(
+        sizeOfSignaturesAndAddresses, _attestations.signaturesOrAddresses.length
+      )
+    );
+
     return addresses;
   }
 

l1-contracts/src/core/libraries/rollup/ValidatorSelectionLib.sol

@@ -235,6 +235,10 @@ library ValidatorSelectionLib {
       proposer = committee[proposerIndex];
 
       setCachedProposer(_slot, proposer, proposerIndex);
+    } else {
+      // Assert that the size of the attestations is as expected, to avoid memory abuse on sizes.
+      // These checks are also performed inside `reconstructCommitteeFromSigners`.
+      _attestations.assertSizes(getStorage().targetCommitteeSize);
     }
 
     // We check that the proposer agrees with the proposal by checking that he attested to it. If we fail to get