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

9 stars 4 forks source link

Reentrancy in ERC777 tokens in collateralTokens.sol #578

Closed c4-bot-1 closed 5 months ago

c4-bot-1 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/CollateralTracker.sol#L424 https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/CollateralTracker.sol#L417 https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/CollateralTracker.sol#L477

Vulnerability details

Impact

Re-entrancy in deposit and mint for ERC777 tokens leads to minting more than expected tokens

Proof of Concept

    function deposit(uint256 assets, address receiver) external returns (uint256 shares) {
        .
        .
        // in return for the shares to be minted
        SafeTransferLib.safeTransferFrom(
            s_underlyingToken,
            msg.sender,
            address(s_panopticPool),
            assets
        );

        // mint collateral shares of the Panoptic Pool funds (this ERC20 token)
        _mint(receiver, shares);

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

Here we see the checks-effects-interactions pattern not followed and in case of ERC777 tokens can be fatal as _mint can be called many times

Tools Used

Manual analysis

Recommended Mitigation Steps

Add noReentrant modifier or follow checks effects interactions patterns like withdraw and redeem

Assessed type

Reentrancy

c4-judge commented 5 months ago

Picodes marked the issue as unsatisfactory: Out of scope

Picodes commented 5 months ago

As said in the readme ERC777 are out of scope https://github.com/code-423n4/2024-04-panoptic/tree/833312ebd600665b577fbd9c03ffa0daf250ed24