Deposit to Zero/Contract Address Leads To Indefinitely Lock the Rewards

[code423n4]

Lines of code

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityFarming.sol#L156

Vulnerability details

Impact

During the code review, It has been observed that, on the NFT staking user can define "to" address with zero/contract address. When the user interface,

Proof of Concept

1. Navigate to the following contract.

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityFarming.sol#L156

    function _sendRewardsForNft(uint256 _nftId, address payable _to) internal {
        NFTInfo storage nft = nftInfo[_nftId];
        require(nft.isStaked, "ERR__NFT_NOT_STAKED");

        (address baseToken, , uint256 amount) = lpToken.tokenMetadata(_nftId);
        amount /= liquidityProviders.BASE_DIVISOR();

        PoolInfo memory pool = updatePool(baseToken);
        uint256 pending;
        uint256 amountSent;
        if (amount > 0) {
            pending = ((amount * pool.accTokenPerShare) / ACC_TOKEN_PRECISION) - nft.rewardDebt + nft.unpaidRewards;
            if (rewardTokens[baseToken] == NATIVE) {
                uint256 balance = address(this).balance;
                if (pending > balance) {
                    unchecked {
                        nft.unpaidRewards = pending - balance;
                    }
                    (bool success, ) = _to.call{value: balance}("");
                    require(success, "ERR__NATIVE_TRANSFER_FAILED");
                    amountSent = balance;
                } else {
                    nft.unpaidRewards = 0;
                    (bool success, ) = _to.call{value: pending}("");
                    require(success, "ERR__NATIVE_TRANSFER_FAILED");
                    amountSent = pending;
                }
            } else {
                IERC20Upgradeable rewardToken = IERC20Upgradeable(rewardTokens[baseToken]);
                uint256 balance = rewardToken.balanceOf(address(this));
                if (pending > balance) {
                    unchecked {
                        nft.unpaidRewards = pending - balance;
                    }
                    amountSent = _sendErc20AndGetSentAmount(rewardToken, balance, _to);
                } else {
                    nft.unpaidRewards = 0;
                    amountSent = _sendErc20AndGetSentAmount(rewardToken, pending, _to);
                }
            }
        }
        nft.rewardDebt = (amount * pool.accTokenPerShare) / ACC_TOKEN_PRECISION;
        emit LogOnReward(_msgSender(), baseToken, amountSent, _to);
    }

Scenario

1. User deposit to NFT with zero address.

deposit(1,Zero_address)

2. After the time, User is trying to withdraw rewards for the NFT.

extractRewards(1,UserAddress)

3. User could not extract rewards due to the following check. The user is already set staker address to zero mistakenly. The staker is zero address.

        nft.staker = _to;

Tools Used

None

Recommended Mitigation Steps

Consider to address validation in the related parameter.

require(_to != 0x0000 & _to != address(this),"Can not stake to zero and contract address")

[ankurdubey521]

While I agree this is an issue I don't believe this puts any assets at risk since the protocol is working as expected, therefore this should have a severity rating of 1.

[pauliax]

Agree with the sponsor, this should belong to the QA report of the warden: #112