077.md

0x52

high

BufferBinaryPool#transfer and transferFrom to or from handler breaks token accounting

Summary

BufferBinaryPool overrides _beforeTokenTransfer to track unlocked token balances. It adds an exception for transfers to and from the handler which doesn't update any accounting. Token balance and unlocked token balances are linked and should only be out of sync during the short deposit window, but if there are transfers to or from a handler then the two balances are permanently disconnected, result in the inability to withdraw and loss of funds.

Vulnerability Detail

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;
    }
}

_beforeTokenTransfer does not update unlocked value for either the to or from when isHandler[from] or isHandler[to]. The result is that in this case, unlockedAmount and balance become permanently decoupled. After, it's impossible for these share to be redeemed or withdrawn, causing loss of funds. This is because the handler has the ERC20 tokens but doesn't have an unlocked balance and the user has an unlocked balance but no ERC20 tokens to burn.

Impact

Transfers to or from a handler will decouple unlockedAmount and balance, making it impossible for the transfered shares to be redeemed

Code Snippet

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

Tool used

Manual Review

Recommendation

Remove handler exception. Aside from burning or minting, unlocked balance should always follow ERC20 tokens.

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