BondNFT.claim can be called several times to increase rewards amount and drain all funds

[code423n4]

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/BondNFT.sol#L168-L187

Vulnerability details

Impact

BondNFT.claim can be called several times to increase rewards amount.

Proof of Concept

BondNft.claim function is called to claim bond rewards that have accrued during the time.
https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/BondNFT.sol#L168-L187

    function claim(
        uint _id,
        address _claimer
    ) public onlyManager() returns(uint amount, address tigAsset) {
        Bond memory bond = idToBond(_id);
        require(_claimer == bond.owner, "!owner");
        amount = bond.pending;
        tigAsset = bond.asset;
        unchecked {
            if (bond.expired) {
                uint _pendingDelta = (bond.shares * accRewardsPerShare[bond.asset][epoch[bond.asset]] / 1e18 - bondPaid[_id][bond.asset]) - (bond.shares * accRewardsPerShare[bond.asset][bond.expireEpoch-1] / 1e18 - bondPaid[_id][bond.asset]);
                if (totalShares[bond.asset] > 0) {
                    accRewardsPerShare[bond.asset][epoch[bond.asset]] += _pendingDelta*1e18/totalShares[bond.asset];
                }
            }
            bondPaid[_id][bond.asset] += amount;
        }
        IERC20(tigAsset).transfer(manager, amount);
        emit ClaimFees(tigAsset, amount, _claimer, _id);
    }

If bond is already expired then function distributes the amount that is accrued in next epochs after bond has expired to bonds that are nor expired yet.

 if (bond.expired) {
                uint _pendingDelta = (bond.shares * accRewardsPerShare[bond.asset][epoch[bond.asset]] / 1e18 - bondPaid[_id][bond.asset]) - (bond.shares * accRewardsPerShare[bond.asset][bond.expireEpoch-1] / 1e18 - bondPaid[_id][bond.asset]);
                if (totalShares[bond.asset] > 0) {
                    accRewardsPerShare[bond.asset][epoch[bond.asset]] += _pendingDelta*1e18/totalShares[bond.asset];
                }
            }

The problem is that currently this function can be called as many times as you wish using Lock.claim function.
So that means that accRewardsPerShare[bond.asset][epoch[bond.asset]] will be always increasing.

Attack example.
1.Attacker creates 2 bonds. One is for short period, another is for little bit longer period.
2.When first bond has expired attacker calls claim too many times to increase accRewardsPerShare[bond.asset][epoch[bond.asset]] value to drain all funds from BondNft.
3.After that attacker claims his another bond and recieve all funds that are in BondNft as reward.

Tools Used

VsCode

Recommended Mitigation Steps

Make sure that accRewardsPerShare updating is made only once.
Such check may help if (bond.expired && bond.pending > 0).

[c4-judge]

GalloDaSballo marked the issue as duplicate of #68

[c4-judge]

GalloDaSballo marked the issue as duplicate of #170

[c4-judge]

GalloDaSballo marked the issue as satisfactory