fix: (C4#124) delete pendingOwner on final ownership renounce (#646)

* chore: delete pendingOwner on final ownership renouncing

* chore: run prettier

* test: add foundry tests

* chore: apply suggested changes

---------

Co-authored-by: Jean Cvllr <31145285+CJ42@users.noreply.github.com>

contracts/LSP14Ownable2Step/LSP14Ownable2Step.sol

@@ -192,6 +192,7 @@ abstract contract LSP14Ownable2Step is ILSP14Ownable2Step, OwnableUnset {
 
         _setOwner(address(0));
         delete _renounceOwnershipStartedAt;
+        delete _pendingOwner;
         emit OwnershipRenounced();
     }
 }

tests/foundry/LSP14Ownable2Step/TwoStepRenounceOwnership.sol

@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity ^0.8.13;
+
+import "forge-std/Test.sol";
+import "@erc725/smart-contracts/contracts/constants.sol";
+import "../../../contracts/LSP0ERC725Account/LSP0ERC725Account.sol";
+
+contract Implementation {
+    // _pendingOwner is at slot 3 for LSP0ERC725Account
+    bytes32[3] __gap;
+    address _pendingOwner;
+
+    function setPendingOwner(address newPendingOwner) external {
+        _pendingOwner = newPendingOwner;
+    }
+}
+
+contract TwoStepRenounceOwnershipTest is Test {
+    LSP0ERC725Account account;
+
+    function setUp() public {
+        // Deploy LSP0 account with this address as owner
+        account = new LSP0ERC725Account(address(this));
+    }
+
+    function testCannotRegainOwnershipAfterRenounce() public {
+        // Call renounceOwnership() to initiate the process
+        account.renounceOwnership();
+
+        // Overwrite _pendingOwner using a delegatecall
+        Implementation implementation = new Implementation();
+        account.execute(
+            OPERATION_4_DELEGATECALL,
+            address(implementation),
+            0,
+            abi.encodeWithSelector(
+                Implementation.setPendingOwner.selector,
+                address(this)
+            )
+        );
+
+        // _pendingOwner is now set to this address
+        assertEq(account.pendingOwner(), address(this));
+
+        // Call renounceOwnership() again to renounce ownership
+        vm.roll(block.number + 200);
+        account.renounceOwnership();
+
+        // Owner is now set to address(0)
+        assertEq(account.owner(), address(0));
+
+        // Call acceptOwnership() to regain ownership should fail
+        // as pendingOwner should be deleted on the second call of renounceOwnership again
+        vm.expectRevert("LSP14: caller is not the pendingOwner");
+        account.acceptOwnership();
+    }
+}