[VAULTS] Various audit fixes

contracts/0.4.24/StETH.sol

@@ -92,6 +92,11 @@ contract StETH is IERC20, Pausable {
     bytes32 internal constant TOTAL_SHARES_POSITION_LOW128 =
         0x6038150aecaa250d524370a0fdcdec13f2690e0723eaf277f41d7cae26b359e6;
 
+    /**
+     * @dev Bitmask for high 128 bits of 256-bit slot
+     */
+    uint256 constant internal UINT128_HIGH_MASK = ~uint256(0) << 128;
+
     /**
       * @notice An executed shares transfer from `sender` to `recipient`.
       *
@@ -515,6 +520,8 @@ contract StETH is IERC20, Pausable {
         require(_recipient != address(this), "MINT_TO_STETH_CONTRACT");
 
         newTotalShares = _getTotalShares().add(_sharesAmount);
+        require(newTotalShares & UINT128_HIGH_MASK == 0, "SHARES_OVERFLOW");
+
         TOTAL_SHARES_POSITION_LOW128.setLowUint128(newTotalShares);
 
         shares[_recipient] = shares[_recipient].add(_sharesAmount);

contracts/0.4.24/utils/UnstructuredStorageExt.sol

@@ -13,9 +13,9 @@ library UnstructuredStorageExt {
     using UnstructuredStorage for bytes32;
 
     uint256 constant internal UINT128_LOW_MASK = ~uint128(0);
-    uint256 constant internal UINT128_HIGH_MASK = UINT128_LOW_MASK << 128;
+    uint256 constant internal UINT128_HIGH_MASK = ~uint256(0) << 128;
     uint256 constant internal UINT160_LOW_MASK = ~uint160(0);
-    uint256 constant internal UINT96_HIGH_MASK = UINT160_LOW_MASK << 160;
+    uint256 constant internal UINT96_HIGH_MASK = ~uint256(0) << 160;
 
     function getLowUint128(bytes32 position) internal view returns (uint256) {
         return position.getStorageUint256() & UINT128_LOW_MASK;

contracts/0.8.25/vaults/VaultHub.sol

@@ -111,6 +111,7 @@ contract VaultHub is PausableUntilWithRoles {
     // -----------------------------
     //           CONSTANTS
     // -----------------------------
+    // some constants are immutables to save bytecode
 
     // keccak256(abi.encode(uint256(keccak256("Lido.Vaults.VaultHub")) - 1)) & ~bytes32(uint256(0xff))
     bytes32 private constant STORAGE_LOCATION = 0x9eb73ffa4c77d08d5d1746cf5a5e50a47018b610ea5d728ea9bd9e399b76e200;
@@ -132,14 +133,16 @@ contract VaultHub is PausableUntilWithRoles {
     uint256 public constant CONNECT_DEPOSIT = 1 ether;
     /// @notice The time delta for report freshness check
     uint256 public constant REPORT_FRESHNESS_DELTA = 2 days;
+
     /// @dev basis points base
-    uint256 internal immutable TOTAL_BASIS_POINTS = 100_00;
+    uint256 internal constant TOTAL_BASIS_POINTS = 100_00;
     /// @dev special value for `disconnectTimestamp` storage means the vault is not marked for disconnect
-    uint48 internal immutable DISCONNECT_NOT_INITIATED = type(uint48).max;
+    uint48 internal constant DISCONNECT_NOT_INITIATED = type(uint48).max;
     /// @notice minimum amount of ether that is required for the beacon chain deposit
     /// @dev used as a threshold for the beacon chain deposits pause
-    uint256 internal immutable MIN_BEACON_DEPOSIT = 1 ether;
-
+    uint256 internal constant MIN_BEACON_DEPOSIT = 1 ether;
+    /// @dev amount of ether required to activate a validator after PDG
+    uint256 internal constant PDG_ACTIVATION_DEPOSIT = 31 ether;
 
     // -----------------------------
     //           IMMUTABLES
@@ -350,11 +353,12 @@ contract VaultHub is PausableUntilWithRoles {
 
         if (!IVaultFactory(LIDO_LOCATOR.vaultFactory()).deployedVaults(_vault)) revert VaultNotFactoryDeployed(_vault);
         IStakingVault vault_ = IStakingVault(_vault);
+        _requireSender(vault_.owner());
         if (vault_.pendingOwner() != address(this)) revert VaultHubNotPendingOwner(_vault);
         if (IPinnedBeaconProxy(address(vault_)).isOssified()) revert VaultOssified(_vault);
         if (vault_.depositor() != address(_predepositGuarantee())) revert PDGNotDepositor(_vault);
         // we need vault to match staged balance with pendingActivations
-        if (vault_.stagedBalance() != _predepositGuarantee().pendingActivations(vault_) * 31 ether) {
+        if (vault_.stagedBalance() != _predepositGuarantee().pendingActivations(vault_) * PDG_ACTIVATION_DEPOSIT) {
             revert InsufficientStagedBalance(_vault);
         }
 
@@ -639,7 +643,7 @@ contract VaultHub is PausableUntilWithRoles {
             // by the Accounting Oracle during the report
             _storage().badDebtToInternalize = _storage().badDebtToInternalize.withValueIncrease({
                 _consensus: CONSENSUS_CONTRACT,
-                _increment: uint104(badDebtToInternalize_)
+                _increment: SafeCast.toUint104(badDebtToInternalize_)
             });
 
             emit BadDebtWrittenOffToBeInternalized(_badDebtVault, badDebtToInternalize_);

contracts/0.8.25/vaults/dashboard/Dashboard.sol

@@ -247,9 +247,9 @@ contract Dashboard is NodeOperatorFee {
     }
 
     /**
-     * @notice Accepts the ownership over the StakingVault transferred from VaultHub on disconnect
-     * and immediately transfers it to a new pending owner. This new owner will have to accept the ownership
-     * on the StakingVault contract.
+     * @notice Accepts the ownership over the disconnected StakingVault transferred from VaultHub
+     *         and immediately passes it to a new pending owner. This new owner will have to accept the ownership
+     *         on the StakingVault contract.
      * @param _newOwner The address to transfer the StakingVault ownership to.
      */
     function abandonDashboard(address _newOwner) external {

contracts/0.8.25/vaults/dashboard/Permissions.sol

@@ -310,7 +310,7 @@ abstract contract Permissions is AccessControlConfirmable {
     }
 
     /**
-     * @dev Checks the confirming roles and sets the owner on the StakingVault.
+     * @dev Checks the confirming roles and transfer the ownership of the vault without disconnecting it from the hub
      * @param _newOwner The address to set the owner to.
      */
     function _transferVaultOwnership(address _newOwner) internal {

contracts/0.8.9/sanity_checks/OracleReportSanityChecker.sol

@@ -60,6 +60,8 @@ struct LimitsList {
 
     /// @notice The max annual increase of the total validators' balances on the Consensus Layer
     ///     since the previous oracle report
+    /// (the increase that is limited does not include fresh deposits to the Beacon Chain as well as withdrawn ether)
+    ///
     /// @dev Represented in the Basis Points (100% == 10_000)
     uint256 annualBalanceIncreaseBPLimit;
 

test/0.4.24/steth.test.ts

@@ -495,4 +495,28 @@ describe("StETH.sol:non-ERC-20 behavior", () => {
       );
     });
   });
+
+  context("_mintShares", () => {
+    it("Reverts when minting to zero address", async () => {
+      await expect(steth.harness__mintShares(ZeroAddress, 1000n)).to.be.revertedWith("MINT_TO_ZERO_ADDR");
+    });
+
+    it("Reverts when minting to stETH contract", async () => {
+      await expect(steth.harness__mintShares(steth, 1000n)).to.be.revertedWith("MINT_TO_STETH_CONTRACT");
+    });
+
+    it("Reverts when minting shares overflow 128 bits", async () => {
+      await expect(steth.harness__mintShares(holder, 2n ** 128n)).to.be.revertedWith("SHARES_OVERFLOW");
+    });
+
+    it("Reverts when minting shares overflow 256 bits", async () => {
+      await expect(steth.harness__mintShares(holder, 2n ** 256n - 1n)).to.be.revertedWith("MATH_ADD_OVERFLOW");
+    });
+
+    it("Mints shares to the recipient", async () => {
+      const balanceOfHolderBefore = await steth.balanceOf(holder);
+      await expect(steth.harness__mintShares(holder, 1000n)).to.not.be.reverted;
+      expect(await steth.sharesOf(holder)).to.equal(balanceOfHolderBefore + 1000n);
+    });
+  });
 });

test/0.8.25/vaults/vaulthub/vaulthub.hub.test.ts

@@ -701,6 +701,13 @@ describe("VaultHub.sol:hub", () => {
       await vault.connect(user).transferOwnership(vaultHub);
     });
 
+    it("reverts if called by non-owner", async () => {
+      await expect(vaultHub.connect(stranger).connectVault(vault)).to.be.revertedWithCustomError(
+        vaultHub,
+        "NotAuthorized",
+      );
+    });
+
     it("reverts if vault is not factory deployed", async () => {
       const randomVault = certainAddress("randomVault");
       await expect(vaultHub.connect(user).connectVault(randomVault))
@@ -711,9 +718,10 @@ describe("VaultHub.sol:hub", () => {
     it("reverts if vault is already connected", async () => {
       const { vault: connectedVault } = await createAndConnectVault(vaultFactory);
 
-      await expect(vaultHub.connect(user).connectVault(connectedVault))
-        .to.be.revertedWithCustomError(vaultHub, "VaultHubNotPendingOwner")
-        .withArgs(connectedVault);
+      await expect(vaultHub.connect(user).connectVault(connectedVault)).to.be.revertedWithCustomError(
+        vaultHub,
+        "NotAuthorized",
+      );
     });
 
     it("connects the vault", async () => {

test/deploy/vaults.ts

@@ -76,7 +76,7 @@ async function createMockStakingVaultAndConnect(
 
   await operatorGridMock.changeVaultTierParams(vault, { ...TIER_PARAMS, ...tierParams });
   await vault.connect(owner).transferOwnership(vaultHub);
-  await vaultHub.connect(deployer).connectVault(vault);
+  await vaultHub.connect(owner).connectVault(vault);
 
   return vault;
 }