code-423n4 / 2024-06-panoptic-findings

1 stars 0 forks source link

After EIP-3074 owners would be unable to withdraw due to the `msg.sender != owner` check #27

Closed howlbot-integration[bot] closed 1 month ago

howlbot-integration[bot] commented 1 month ago

Lines of code

https://github.com/code-423n4/2024-06-panoptic/blob/153f0d82440b7e63075d55b0659706531431145f/contracts/CollateralTracker.sol#L513-L548

Vulnerability details

Proof of Concept

Take a look at https://github.com/code-423n4/2024-06-panoptic/blob/153f0d82440b7e63075d55b0659706531431145f/contracts/CollateralTracker.sol#L513-L548

    function withdraw(
        uint256 assets,
        address receiver,
        address owner
    ) external returns (uint256 shares) {
        if (assets > maxWithdraw(owner)) revert Errors.ExceedsMaximumRedemption();

        shares = previewWithdraw(assets);

        // check/update allowance for approved withdraw
        if (msg.sender != owner) {
            uint256 allowed = allowance[owner][msg.sender]; // Saves gas for limited approvals.

            if (allowed != type(uint256).max) allowance[owner][msg.sender] = allowed - shares;
        }

        // burn collateral shares of the Panoptic Pool funds (this ERC20 token)
        _burn(owner, shares);

        // update tracked asset balance
        unchecked {
            s_poolAssets -= uint128(assets);
        }

        // transfer assets (underlying token funds) from the PanopticPool to the LP
        SafeTransferLib.safeTransferFrom(
            s_underlyingToken,
            address(s_panopticPool),
            receiver,
            assets
        );

        emit Withdraw(msg.sender, receiver, owner, assets, shares);

        return shares;
    }

This function is used to withdraw assests to the receiver by the owner, issue however is that the function includes a check that if (msg.sender != owner) which would cause for the attempt at withdrawing to fail after the implementation of EIP 3074

Now would be key to note that EIP-3074 has officially been included in the next Ethereum hard fork Pectra upgrade (Prague upgrade for short) ... source, so we can conclude that the new functionalities from the EIP proposal has been finalized and is to be adopted, so all current withdrawal attempts by integrators/users that actively decide use the new opcodes introduced with the EIP would not be able to have their withdrawals executed.

Impact

Availability of protocol's core functionality is affected, which would suffice as medium based on Code4rena's severity categorization.

Recommended Mitigation Steps

Consider reimplementing the check to ensure users making use of the EIP can still normally integrate with protocol.

Assessed type

Context

c4-judge commented 1 month ago

Picodes marked the issue as unsatisfactory: Invalid