sherlock-audit / 2024-06-boost-aa-wallet-judging

3 stars 1 forks source link

TessKimy - Fee on transfer tokens are not supported in boost protocol #419

Open sherlock-admin4 opened 2 months ago

sherlock-admin4 commented 2 months ago

TessKimy

Medium

Fee on transfer tokens are not supported in boost protocol

Summary

Fee on transfer tokens are not supported due to wrong validation check in allocate() function

Root Cause

In budget contracts, allocate function is used in order to store the funds and use them as reward in incentives.

Based on the README, fee on transfer tokens should be supported by the protocol:

Protocol should work with all native and ERC20 tokens that adhear to standard including weird tokens. No assumptions are made in regards to the properties of integrating tokens and weird trait tokens should be handled normally. Specifically we'd expect to interact with USDT and other stable coins/decimal 6 coins and erc20z from Zora as well as other fractional NFT contracts as long as they adhere to ERC20 standard. We aren't whitelisting tokens so any erc20 should work (including weird ones).

In following allocate() function validation makes it impossible:

    function allocate(bytes calldata data_) external payable virtual override returns (bool) {
        Transfer memory request = abi.decode(data_, (Transfer));
        if (request.assetType == AssetType.ETH) {
            FungiblePayload memory payload = abi.decode(request.data, (FungiblePayload));

            // Ensure the value received is equal to the `payload.amount`
            if (msg.value != payload.amount) {
                revert InvalidAllocation(request.asset, payload.amount);
            }
        } else if (request.assetType == AssetType.ERC20) {
            FungiblePayload memory payload = abi.decode(request.data, (FungiblePayload));

            // Transfer `payload.amount` of the token to this contract
            request.asset.safeTransferFrom(request.target, address(this), payload.amount);
&>          if (request.asset.balanceOf(address(this)) < payload.amount) {
                revert InvalidAllocation(request.asset, payload.amount);
            }
        } else if (request.assetType == AssetType.ERC1155) {
            ERC1155Payload memory payload = abi.decode(request.data, (ERC1155Payload));

            // Transfer `payload.amount` of `payload.tokenId` to this contract
            IERC1155(request.asset).safeTransferFrom(
                request.target, address(this), payload.tokenId, payload.amount, payload.data
            );
            if (IERC1155(request.asset).balanceOf(address(this), payload.tokenId) < payload.amount) {
                revert InvalidAllocation(request.asset, payload.amount);
            }
        } else {
            // Unsupported asset type
            return false;
        }

        return true;
    }

In fee on transfer tokens after a transfer action balance of the contract will be less than payload amount. In conclusion allocate function will always revert while using fee on transfer tokens.

Impact

Medium - All the ERC20 tokens ( even weird ones ) is in the scope and it's impossible to allocate a budget with these tokens. Medium should be appropriate severity for this issue.

PoC

No response

Mitigation

Removing the validation, checking balance difference between the transactions and using this as a payload amount will solve the problem.