code-423n4 / 2022-05-backd-findings

0 stars 0 forks source link

In BkdLocker codes uses safeTransferFrom() multiple times for reward tokens without considering deflationary tokens #167

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/BkdLocker.sol#L90-L100 https://github.com/code-423n4/2022-05-backd/blob/2a5664d35cde5b036074edef3c1369b984d10010/protocol/contracts/BkdLocker.sol#L227-L232

Vulnerability details

Impact

if the token is deflationary then contract will receive less token that requested amount but contract don't check for the real transferred amount so because this is happening in reward token and gov token then those logics can be calculated wrongly and some users don't receive rewards.

Proof of Concept

This is where contract transferees rewards token and increase reward amount:

    function depositFees(uint256 amount) external override {
        require(amount > 0, Error.INVALID_AMOUNT);
        require(totalLockedBoosted > 0, Error.NOT_ENOUGH_FUNDS);
        IERC20(rewardToken).safeTransferFrom(msg.sender, address(this), amount);

        RewardTokenData storage curRewardTokenData = rewardTokenData[rewardToken];

        curRewardTokenData.feeIntegral += amount.scaledDiv(totalLockedBoosted);
        curRewardTokenData.feeBalance += amount;
        emit FeesDeposited(amount);
    }

As you can see it assumes amount token is received and increase curRewardTokenData.feeIntegral based on requested amount not real transferred reward tokens.

Tools Used

VIM

Recommended Mitigation Steps

check the real transferred amounts.

chase-manning commented 2 years ago

We do not support deflationary tokens

GalloDaSballo commented 2 years ago

Given the setup we were offered with:

We can downgrade the severity of the finding to non-critical.

The code technically can be broken by fee/deflationary tokens, and end users should verify the configuration, however the contest as well as the sponsor lead us to believe that the tokens used will not break that invariant