Accounted balance of GlpStrategy does not match withdrawable balance, allowing for attackers to steal unclaimed rewards

[code423n4]

Lines of code

https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/glp/GlpStrategy.sol#L129

Vulnerability details

Impact

Attackers can steal unclaimed rewards due to insufficient accounting.

Proof of Concept

Pricing of shares for Yieldbox strategies is dependent upon the total underlying balance of the strategy. We can see below how we mint an amount of shares according to this underlying amount.

// depositAsset()
uint256 totalAmount = _tokenBalanceOf(asset);
if (share == 0) {
    // value of the share may be lower than the amount due to rounding, that's ok
    share = amount._toShares(totalSupply[assetId], totalAmount, false);
} else {
    // amount may be lower than the value of share due to rounding, in that case, add 1 to amount (Always round up)
    amount = share._toAmount(totalSupply[assetId], totalAmount, true);
}

_mint(to, assetId, share);

The total underlying balance of the strategy is obtained via asset.strategy.currentBalance.

function _tokenBalanceOf(Asset storage asset) internal view returns (uint256 amount) {
    return asset.strategy.currentBalance();
}

GlpStrategy._currentBalance does not properly track all unclaimed rewards.

function _currentBalance() internal view override returns (uint256 amount) {
    // This _should_ included both free and "reserved" GLP:
    amount = IERC20(contractAddress).balanceOf(address(this));
}

As a result, attackers can:

• Deposit a high amount when there are unclaimed rewards
  • Receiving a higher amount of shares than they would if accounting included unclaimed rewards
  • Harvests unclaimed rewards, increasing _currentBalance, only after they received shares
• Withdraw all shares
  • Now that the balance is updated to include previously unclaimed rewards, the attacker profits their relative share of the unclaimed rewards
    • The more the attacker deposits relative to the strategy balance, the greater proportion of interest they receive

Recommended Mitigation Steps

It's recommended that _currentBalance include some logic to retrieve the amount and value of unclaimed rewards to be included in it's return value.

Assessed type

Other

[c4-pre-sort]

minhquanym marked the issue as primary issue

[c4-sponsor]

cryptolyndon marked the issue as sponsor confirmed

[c4-judge]

dmvt marked the issue as selected for report