code-423n4 / 2024-02-spectra-findings

4 stars 2 forks source link

Users will not be able to call `PrincipalToken::withdraw` and `PrincipalToken::redeem` functions when the contract is paused #241

Closed c4-bot-5 closed 9 months ago

c4-bot-5 commented 9 months ago

Lines of code

https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/tokens/PrincipalToken.sol#L828 https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/tokens/PrincipalToken.sol#L828

Vulnerability details

Impact

In unusual circumstances when the protocol is paused, users will not be able to withdraw their assets, which they would want to.

Proof of Concept

Add the following test in PrincipalToken.t.sol to observe the proof.

    function testWithdrawWhenPaused() public {
        uint256 amount = 1e18;
        underlying.approve(address(principalToken), amount);
        principalToken.deposit(amount, testUser);

        uint256 beforeBalance = underlying.balanceOf(testUser);

        vm.prank(scriptAdmin);
        principalToken.pause();
        principalToken.withdraw(amount, testUser, testUser); // this will revert because the protocol was paused
    }

This test will fail with EnforcedPause error.

Tools Used

Manual review

Recommended Mitigation Steps

-   function _beforeWithdraw(uint256 _assets, address _owner) internal whenNotPaused nonReentrant {
+   function _beforeWithdraw(uint256 _assets, address _owner) internal nonReentrant {
        ...
    }

-    function _beforeRedeem(uint256 _shares, address _owner) internal nonReentrant whenNotPaused {
+    function _beforeRedeem(uint256 _shares, address _owner) internal nonReentrant {
        ...
    }

-    function maxWithdraw(address owner) public view override whenNotPaused returns (uint256) 
+    function maxWithdraw(address owner) public view override returns (uint256) {
        ...
     }

Assessed type

Other

c4-pre-sort commented 9 months ago

gzeon-c4 marked the issue as duplicate of #167

c4-pre-sort commented 9 months ago

gzeon-c4 marked the issue as insufficient quality report

c4-judge commented 8 months ago

JustDravee marked the issue as unsatisfactory: Invalid