fix: Possible ignored overflow in _lock

lidofinance · Aug 20, 2024 · 583338b · 583338b
1 parent b53679a
commit 583338b
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 9 deletions.
diff --git a/src/abstract/CSBondLock.sol b/src/abstract/CSBondLock.sol
@@ -105,16 +105,14 @@ abstract contract CSBondLock is ICSBondLock, Initializable {
         if (amount == 0) {
             revert InvalidBondLockAmount();
         }
-        unchecked {
-            if ($.bondLock[nodeOperatorId].retentionUntil > block.timestamp) {
-                amount += $.bondLock[nodeOperatorId].amount;
-            }
-            _changeBondLock({
-                nodeOperatorId: nodeOperatorId,
-                amount: amount,
-                retentionUntil: block.timestamp + $.bondLockRetentionPeriod
-            });
+        if ($.bondLock[nodeOperatorId].retentionUntil > block.timestamp) {
+            amount += $.bondLock[nodeOperatorId].amount;
         }
+        _changeBondLock({
+            nodeOperatorId: nodeOperatorId,
+            amount: amount,
+            retentionUntil: block.timestamp + $.bondLockRetentionPeriod
+        });
     }
 
     /// @dev Reduce locked bond amount for the given Node Operator without changing retention period

diff --git a/test/CSAccounting.t.sol b/test/CSAccounting.t.sol
@@ -3630,6 +3630,18 @@ contract CSAccountingLockBondETHTest is CSAccountingBaseTest {
         accounting.lockBondETH(0, 1 ether);
     }
 
+    function test_lockBondETH_RevertWhen_LockOverflow() public {
+        mock_getNodeOperatorsCount(1);
+
+        vm.prank(address(stakingModule));
+        accounting.lockBondETH(0, 1 ether);
+        assertEq(accounting.getActualLockedBond(0), 1 ether);
+
+        vm.expectRevert();
+        vm.prank(address(stakingModule));
+        accounting.lockBondETH(0, type(uint256).max);
+    }
+
     function test_releaseLockedBondETH() public assertInvariants {
         mock_getNodeOperatorsCount(1);