097.md

Ch_301

medium

The liquidity providers can’t keep their BLP save

Summary

The handler is a RewardRouter Contract (Forked from the GMX Staking contracts) which is not part of the Audit Scope as the team said. Only the DEFAULT_ADMIN_ROLE can add or remove handlers from isHandler[ ], so you need to trust the Admin but this is not the main case

Vulnerability Detail

You can't just trust the handler to transfer BLP from the investors On BufferBinaryPool.transferFrom()

        if (isHandler[msg.sender]) {
            _transfer(_sender, _recipient, _amount);
            return true;
        }

As we can see the handler can transfer any amount from/to any user

Also (The scenario is not supposed to be, but it is possible) Lat’s say a handler provider supplies tokenX to the pool by invoking BufferBinaryPool.provide() ==> _provide(), so he will be registered on the liquidityPerUser mapping. After lockupPeriod he will have some unlockedAmount Now the handler could invoke transferFrom() or transfer() directly The _transfer() has an open hook _beforeTokenTransfer() which is supposed to update the liquidityPerUser mapping and invoke _updateLiquidity()

function _beforeTokenTransfer(
        address from,
        address to,
        uint256 value
    ) internal override {
        if (!isHandler[from] && !isHandler[to] && from != address(0)) {
            _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;
        }
    }

But it has a check in case the handler is part of this transaction from/to, it just skipped this hook and transferred the BLP tokens Now both the sender and the receiver (there is some more scenario here in case the receiver transfer the tokens to another account ) can’t invoke BufferBinaryPool.withdraw() so the tokenX amount will be locked in the pool

Impact

• The handler can transfer any amount of BLP from any user
• tokenX could be locked in the pool

Code Snippet

    function transferFrom(
        address _sender,
        address _recipient,
        uint256 _amount
    ) public virtual override returns (bool) {
        if (isHandler[msg.sender]) {
            _transfer(_sender, _recipient, _amount);
            return true;
        }

https://github.com/bufferfinance/Buffer-Protocol-v2/blob/83d85d9b18f1a4d09c728adaa0dde4c37406dfed/contracts/core/BufferBinaryPool.sol#L74-L82

function _beforeTokenTransfer(
        address from,
        address to,
        uint256 value
    ) internal override {
        if (!isHandler[from] && !isHandler[to] && from != address(0)) {
            _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;
        }
    }

https://github.com/bufferfinance/Buffer-Protocol-v2/blob/83d85d9b18f1a4d09c728adaa0dde4c37406dfed/contracts/core/BufferBinaryPool.sol#L338-L352

Tool used

Manual Review

Recommendation

Check the if _amount == allowance(_sender, msg.sender).Or you can add a ** time lock**.