137.md

bin2chen

medium

BufferBinaryPool.sol Handler may can't withdraw

Summary

_withdraw() will limit user#unlockedAmount >= burn, if the user is a handler may never be able to _withdraw()

Vulnerability Detail

Suppose alice is a Handler bob transfers 10 "Buffer LP Token" to alice Because BufferBinaryPool#_beforeTokenTransfer() will skip all handlers, so alice#unlockedAmount is still equal to 0, but the actual balance is 10 At this point alice wants to withdraw(10), but in #_withdraw() will check user#unlockedAmount >= 10, resulting in withdraw will fail. alice can transfer the 10 tokens to others user, so that others user can withdraw() although it is more trouble but If alice is canceled "Handler" before transfer, so that the transfer will also fail, because _beforeTokenTransfer() will fail, so you can not transfer, can not withdraw, locking token

    function _beforeTokenTransfer(
        address from,
        address to,
        uint256 value
    ) internal override {
        if (!isHandler[from] && !isHandler[to] && from != address(0)) { //****@audit skip all hander***/
            _updateLiquidity(from);
            require(
                liquidityPerUser[from].unlockedAmount >= value,
                "Pool: Transfer of funds in lock in period is blocked"
            );
            liquidityPerUser[from].unlockedAmount -= value;
            liquidityPerUser[to].unlockedAmount += value;
        }
    }

    function _withdraw(uint256 tokenXAmount, address account)
        internal
        returns (uint256 burn)
    {
...
        require(
            liquidityPerUser[account].unlockedAmount >= burn, //****@audit alice will fail , because unlockedAmount==0, but balance=10
            "Pool: Withdrawal amount is greater than current unlocked amount"
        );

Impact

Handler may can't withdraw

Code Snippet

https://github.com/sherlock-audit/2022-11-buffer/blob/main/contracts/contracts/core/BufferBinaryPool.sol#L314

Tool used

Manual Review

Recommendation

    function _withdraw(uint256 tokenXAmount, address account)
        internal
        returns (uint256 burn)
    {
...
        require(
+          isHandler[account] || //*** burn will check Have enough balance  , so it ok/
            liquidityPerUser[account].unlockedAmount >= burn, 
            "Pool: Withdrawal amount is greater than current unlocked amount"
        );
...

    function setHandler(address _handler, bool _isActive)
        external
        onlyRole(DEFAULT_ADMIN_ROLE)
    {
+      if (isHandler[_handler] && !_isActive){ //if cancel hander,need process unlockedAmount value
+           liquidityPerUser[_handler].unlockedAmount = balanceOf(_handler);
+           liquidityPerUser[_handler].nextIndexForUnlock = liquidityPerUser[_handler].lockedAmounts.length;            
+       }
        isHandler[_handler] = _isActive;
    }