Skip to content
This repository was archived by the owner on Mar 24, 2024. It is now read-only.
This repository was archived by the owner on Mar 24, 2024. It is now read-only.

0xbepresent - Pool's strategies does not support fee on transfer tokens causing an error in the counting system #536

@sherlock-admin

Description

@sherlock-admin

0xbepresent

medium

Pool's strategies does not support fee on transfer tokens causing an error in the counting system

There is not support on fee on transfer tokens when funding a pool causing an error in the counting system.

Vulnerability Detail

The Allo::_fundPool() function helps to depositants to send tokens to the _strategy. The problem is that pools can use fee on transfer tokens, so when the transfer transaction occurs the current amount sent to the strategy could be less.

The transfer occurs in the code line 516 using the amount amountAfterFee, then in the code line 517 the strategy amount is increased:

File: Allo.sol
502:     function _fundPool(uint256 _amount, uint256 _poolId, IStrategy _strategy) internal {
503:         uint256 feeAmount;
504:         uint256 amountAfterFee = _amount;
505: 
506:         Pool storage pool = pools[_poolId];
507:         address _token = pool.token;
508: 
509:         if (percentFee > 0) {
510:             feeAmount = (_amount * percentFee) / getFeeDenominator();
511:             amountAfterFee -= feeAmount;
512: 
513:             _transferAmountFrom(_token, TransferData({from: msg.sender, to: treasury, amount: feeAmount}));
514:         }
515: 
516:         _transferAmountFrom(_token, TransferData({from: msg.sender, to: address(_strategy), amount: amountAfterFee}));
517:         _strategy.increasePoolAmount(amountAfterFee);
518: 
519:         emit PoolFunded(_poolId, amountAfterFee, feeAmount);
520:     }

The BaseStrategy::increasePoolAmount() function is not considering the fee charged by the token.

Impact

There are errors in the counting system (poolAmount) that can affect the distribution of the tokens to the recipients because the calculation can be for a certain amount but actually there could be a less amount in the pools. E.g. the QVBaseStrategy::_getPayout() calculations may be using more tokens than the available in the pool.

Code Snippet

Tool used

Manual review

Recommendation

Calculates the strategy balance before and after the transfer transaction:

   function _fundPool(uint256 _amount, uint256 _poolId, IStrategy _strategy) internal {
        uint256 feeAmount;
        uint256 amountAfterFee = _amount;

        Pool storage pool = pools[_poolId];
        address _token = pool.token;

        if (percentFee > 0) {
            feeAmount = (_amount * percentFee) / getFeeDenominator();
            amountAfterFee -= feeAmount;

            _transferAmountFrom(_token, TransferData({from: msg.sender, to: treasury, amount: feeAmount}));
        }
++      uint256 strategyBalanceBefore = _token.balanceOf(address(strategy));
        _transferAmountFrom(_token, TransferData({from: msg.sender, to: address(_strategy), amount: amountAfterFee}));
++      uint256 strategyBalanceAfter = _token.balanceOf(address(strategy));
--      _strategy.increasePoolAmount(amountAfterFee);
++      _strategy.increasePoolAmount(strategyBalanceAfter - strategyBalanceBefore);

        emit PoolFunded(_poolId, amountAfterFee, feeAmount);
    }

Of course elaborate a better solution that consider NATIVE tokens

Duplicate of #19

Metadata

Metadata

Assignees

No one assigned

    Labels

    DuplicateA valid issue that is a duplicate of an issue with `Has Duplicates` labelMediumA valid Medium severity issueRewardA payout will be made for this issue

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions