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

9 stars 4 forks source link

Insecure use of SafeTransferFrom() function #574

Closed c4-bot-7 closed 5 months ago

c4-bot-7 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/PanopticFactory.sol#L172-L195

Vulnerability details

Impact

The PanopticFactory contract uses safeTransferFrom (in uniswapV3MintCallback function) to transfer tokens from the payer to the sender.

However, safeTransferFrom can't prevent re-entrancy, meaning the function can call itself while it's still "in progress." This can potentially create malicious recursive calls and drain the contract balances.

Proof of Concept

function uniswapV3MintCallback( 
    uint256 amount0Owed, 
    uint256 amount1Owed, 
    bytes calldata data 
) external { 
    // ...
    if (amount0Owed > 0) 
        SafeTransferLib.safeTransferFrom( decoded.poolFeatures.token0, decoded.payer, msg.sender, amount0Owed ); 
    if (amount1Owed > 0) 
        SafeTransferLib.safeTransferFrom( decoded.poolFeatures.token1, decoded.payer, msg.sender, amount1Owed ); 
}

Tools Used

Manual Review

Recommended Mitigation Steps

To mitigate reentrancy risks, it's recommended to adopt the Checks-Effects-Interactions (CEI) pattern, where external contract calls should be the last part of the function, after all checks and state changes. In this case, it would be useful to apply the ReentrancyGuard from the OpenZeppelin library to safeguard these external calls.

Assessed type

Reentrancy

c4-judge commented 5 months ago

Picodes marked the issue as unsatisfactory: Insufficient proof