OpportunitiesLeaderboardAbout Cantina
Puffer Finance

puffer-contracts

Cantina Security Report

Organization

@puffer

Engagement Type

Cantina Reviews

Period

-

Repositories

GitHubPufferFinance/puffer-contracts

Researchers

ladboy233

Sujith Somraaj

Findings

Low Risk

1 findings

1 fixed

0 acknowledged

Informational

2 findings

1 fixed

1 acknowledged

Low Risk1 finding

  1. Incorrect fee calculation in maxWithdraw function

    State

    Fixed

    PR #112

    Severity

    Severity: Low

    Submitted by

    Sujith Somraaj

    Description

    The maxWithdraw() function in PufferVaultV5.sol incorrectly applies exit fees to the vault's total liquidity rather than adding fees to individual withdrawal requests. This leads to an artificial reduction in the reported maximum withdrawal amount, potentially causing confusion and preventing users from withdrawing their full entitled assets.

    Users will be shown a lower maximum withdrawal amount than what the protocol can support. This creates an artificial liquidity constraint that becomes more severe as the exit fee increases. For example, with a 1% exit fee, only around 99% of the actual liquidity would be available for withdrawal.

    The current implementation calculates available liquidity as:

    uint256 availableLiquidity = vaultLiquidity - _feeOnRaw(vaultLiquidity, getExitFeeBasisPoints());

    However, the protocol's withdrawal mechanism adds fees to individual withdrawal amounts rather than reducing the total liquidity pool. This is evident in the previewWithdraw() function, which adds fees to the requested withdrawal amount:

    function previewWithdraw(uint256 assets) public view virtual override returns (uint256) {    uint256 fee = _feeOnRaw(assets, getExitFeeBasisPoints());    return super.previewWithdraw(assets + fee);}

    Proof of Concept

    Place the following test inside test folder:

    import {Test} from "forge-std/Test.sol";import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";import "src/PufferVaultV5.sol";
import "forge-std/console.sol";
interface Upgrade {    function upgradeToAndCall(address newImpl, bytes memory data) external payable;}
contract AuditTest is Test {    PufferVaultV5 public vaultImpl;    PufferVaultV5 public vault;
    function setUp() external {        vm.createSelectFork("https://mainnet.infura.io/v3/<YOUR INFURA KEY>");        vault = PufferVaultV5(payable(0xD9A442856C234a39a81a089C06451EBAa4306a72));        vaultImpl = new PufferVaultV5(IStETH(0xae7ab96520DE3A18E5e111B5EaAb095312D7fE84), ILidoWithdrawalQueue(address(0)), IWETH(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2), IPufferOracleV2(0x0BE2aE0edbeBb517541DF217EF0074FC9a9e994f), IPufferRevenueDepositor(0x21660F4681aD5B6039007f7006b5ab0EF9dE7882));
        vm.startPrank(0x3C28B7c7Ba1A1f55c9Ce66b263B33B204f2126eA);        Upgrade(0xD9A442856C234a39a81a089C06451EBAa4306a72).upgradeToAndCall(address(vaultImpl), "");    }
    function test_maxRedeem() external {        address user = address(420);        deal(vault.asset(), user, 100000e18);                vm.startPrank(user);        ERC20(vault.asset()).approve(address(vault), 100000e18);        vault.deposit(100000e18, user);
        vm.startPrank(address(vault));        ERC20(vault.asset()).transfer(address(0), 100000e18);
        console.log("user balance:", vault.balanceOf(user));        uint256 maxRedeem = vault.maxRedeem(user);        uint256 maxWithdraw = vault.maxWithdraw(user);        console.log("max withdraw:", maxWithdraw);        console.log("max redeem:", maxRedeem);        console.log("preview redeem:", vault.previewRedeem(vault.balanceOf(user)));                vm.startPrank(user);        uint256 assets = vault.withdraw(maxWithdraw, user, user);        console.log("assets:", assets);
        console.log(ERC20(vault.asset()).balanceOf(address(vault)));        console.log(address(vault).balance);    }}

    Using the original code, the user can withdraw 2723.01E from the vault, even though there is more than2750E available in liquidity. This discrepancy arises because the calculation in the maxWithdraw function is flawed. In the same circumstances, if the user uses maxRedeem(), they can redeem the full 2750E.

    Recommendation

    1. The maxWithdraw() function should not subtract fees from the total liquidity. Instead, it should simply compare the user's assets with the vault's total liquidity:
    function maxWithdraw(address owner) public view virtual override returns (uint256 maxAssets) {    uint256 maxUserAssets = previewRedeem(balanceOf(owner));    uint256 vaultLiquidity = (_WETH.balanceOf(address(this)) + (address(this).balance));        // Return the minimum of user's assets and available liquidity    return Math.min(maxUserAssets, vaultLiquidity);}

    This correction ensures that the maximum withdrawal amount is properly calculated, allowing users to withdraw their full entitled assets up to the available liquidity.

    1. Consider adding fuzz tests to ensure all cases are properly covered for the function.

Informational2 findings

  1. PufferModuleManager#customExternalCall does not check if the call data is validated in avsRegistryCoordinator

    State

    Acknowledged

    Severity

    Severity: Informational

    ≈

    Likelihood: Low

    ×

    Impact: Low

    Submitted by

    ladboy233

    Description

    In RestakingOperatorController.sol, the code that trigger external call in restakingOperator validate if the custom call is allowed via checkCustomCallData

    /**     * @notice Custom external call to the restaking operator     * @dev This function can be called by Operator owners     * @param restakingOperator The address of the restaking operator     * @param data The data to call the restaking operator with     */    function customExternalCall(address restakingOperator, bytes calldata data) external payable override {        require(_operatorOwners[restakingOperator] == msg.sender, NotOperatorOwner(restakingOperator, msg.sender));        bytes4 selector = bytes4(data[:4]);        require(_allowedSelectors[selector], NotAllowedSelector(selector));        if (selector == CUSTOM_CALL_SELECTOR) {            _checkCustomCallData(data);        }        (bool success,) = restakingOperator.call{ value: msg.value }(data);        require(success, CustomCallFailed());        emit CustomExternalCall(restakingOperator, data, msg.value);    }

    and

    function _checkCustomCallData(bytes calldata data) private view {        (address avsRegistryCoordinator, bytes memory customCalldata) = abi.decode(data[4:], (address, bytes));        require(            _avsContractsRegistry.isAllowedRegistryCoordinator(avsRegistryCoordinator, customCalldata), Unauthorized()        );    }

    But in PufferModuleManager#customExternalCall, the code that trigger external call in restakingOperator has no such validation.

    The calldata and target that is not allowed in avsRegistryCoordinator can still be executed via PufferModuleManager#customExternalCall

    Recommendation

    Consider add _checkCustomCallData in PufferModuleManager#customExternalCall as well.

  2. Incorrect function signature in CUSTOM_CALL_SELECTOR comment

    State

    Fixed

    https://github.com/PufferFinance/puffer-contracts/pull/113/files

    Severity

    Severity: Informational

    Submitted by

    Sujith Somraaj

    Description

    The comment for the CUSTOM_CALL_SELECTOR constant in the RestakingOperatorController.sol contract contains an incorrect function signature description that does not match the actual selector value.

    Current code:

    bytes4 private constant CUSTOM_CALL_SELECTOR = 0x58fa420c; // bytes4(keccak256("customCalldataCall(address,bytes)(address,bytes,uint256)"))

    The selector value 0x58fa420c corresponds to:

    bytes4(keccak256("customCalldataCall(address,bytes)"))

    The comment incorrectly includes return types and uses an invalid function signature format with separate parentheses for inputs and outputs.

    Recommendation

    Update the comment to correctly document the function signature:

    bytes4 private constant CUSTOM_CALL_SELECTOR = 0x58fa420c; // bytes4(keccak256("customCalldataCall(address,bytes)"))