## [H-05] `ProtocolRewards.getReward()`: Implementation may not let stakers get reward when contracts are underfunded

[code423n4]

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/miner/ProtocolRewardsPool.sol#L197

Vulnerability details

Impact

https://github.com/code-423n4/2023-06-lybra/blob/main/contracts/lybra/miner/ProtocolRewardsPool.sol#L197

function getReward() external updateReward(msg.sender) {
    uint reward = rewards[msg.sender];
    if (reward > 0) {
        rewards[msg.sender] = 0;
        IEUSD EUSD = IEUSD(configurator.getEUSDAddress());
        uint256 balance = EUSD.sharesOf(address(this));
        uint256 eUSDShare = balance >= reward ? reward : reward - balance;
        /// @audit if transferShares revert, getReward() will always revert
        EUSD.transferShares(msg.sender, eUSDShare);
        if(reward > eUSDShare) {
            ERC20 peUSD = ERC20(configurator.peUSD());
            uint256 peUSDBalance = peUSD.balanceOf(address(this));
            if(peUSDBalance >= reward - eUSDShare) {
                peUSD.transfer(msg.sender, reward - eUSDShare);
                emit ClaimReward(msg.sender, EUSD.getMintedEUSDByShares(eUSDShare), address(peUSD), reward - eUSDShare, block.timestamp);
            } else {
                if(peUSDBalance > 0) {
                    peUSD.transfer(msg.sender, peUSDBalance);
                }
                ERC20 token = ERC20(configurator.stableToken());
                uint256 tokenAmount = (reward - eUSDShare - peUSDBalance) * token.decimals() / 1e18;
                token.transfer(msg.sender, tokenAmount);
                emit ClaimReward(msg.sender, EUSD.getMintedEUSDByShares(eUSDShare), address(token), reward - eUSDShare, block.timestamp);
            }
        } else {
            emit ClaimReward(msg.sender, EUSD.getMintedEUSDByShares(eUSDShare), address(0), 0, block.timestamp);
        }

    }
}

• ProtocolRewardsPool.getReward() will always revert if there is insufficient EUSD to send to borrower, never allowing the order of other earnings to be executed. This could possibly occur if EUSD contract is underfunded or if user has a low share balance due to transfers leading to high eUSD share of rewards. This unfairly prevents borrowers from receiving earnings in the form of peUSD or other approved stablecoins.

• In the event that EUSD balance is sufficient to send eUSDShare of rewards is sent to borrower but peUSD or other stable token is underfunded, user claiming rewards will also be reverted and not receive eUSD share of rewards

Vulnerability Details

• ProtocolRewards.getReward() --> EUSD.transferShares() --> reverts if contract have insufficient share balance

• ProtocolRewards.getReward() --> peUSD.transfer fails if peUSD contract has insufficient balance

• ProtocolRewards.getReward() --> token.transfer fails if other stable coin contract has insufficient balance

Tools Used

Manual Analysis

Recommendation

Consider using a try catch block to catch failure of EUSD share transfer. If eusd transfer is unsucessful, cache reward remaining and allow logic of peUSD and other stable coin to occur.

Also consider allowing rewards claimer to claim up to contract balance and cache remaining rewards unclaimed instead of reverting if any of the other reward token contracts are underfunded (eUSD, peUSD, stablecoin).

Assessed type

DoS

[c4-pre-sort]

JeffCX marked the issue as low quality report

[c4-judge]

0xean marked the issue as unsatisfactory: Insufficient quality

[nevillehuang]

@0xean @LybraFinance

In the comments here, it clearly states here that:

the eUSD will be prioritized for distribution. Distributes earnings in the order of peUSD and other stablecoins if the eUSD balance is insufficient.

But in EUSD.transferShares it calls the internal _transferShares here

  function transferShares(address _recipient, uint256 _sharesAmount) public returns (uint256) {
      address owner = _msgSender();
->    _transferShares(owner, _recipient, _sharesAmount);
      emit TransferShares(owner, _recipient, _sharesAmount);
      uint256 tokensAmount = getMintedEUSDByShares(_sharesAmount);
      emit Transfer(owner, _recipient, tokensAmount);
      return tokensAmount;
  }

This will revert here if owner does not have enough shares, which contradicts the above mentioned comment:

  function _transferShares(address _sender, address _recipient, uint256 _sharesAmount) internal {
      require(_sender != address(0), "TRANSFER_FROM_THE_ZERO_ADDRESS");
      require(_recipient != address(0), "TRANSFER_TO_THE_ZERO_ADDRESS");

      uint256 currentSenderShares = shares[_sender];
 ->   require(_sharesAmount <= currentSenderShares, "TRANSFER_AMOUNT_EXCEEDS_BALANCE");

      shares[_sender] = currentSenderShares.sub(_sharesAmount);
      shares[_recipient] = shares[_recipient].add(_sharesAmount);
  }

[0xean]

        uint256 balance = EUSD.sharesOf(address(this));
        uint256 eUSDShare = balance >= reward ? reward : reward - balance;
        /// @audit if transferShares revert, getReward() will always revert
        EUSD.transferShares(msg.sender, eUSDShare);

Please write a coded POC to show how you think this will revert. Otherwise, my judging is finalized here.

[nevillehuang]

        uint256 balance = EUSD.sharesOf(address(this));
        uint256 eUSDShare = balance >= reward ? reward : reward - balance;
        /// @audit if transferShares revert, getReward() will always revert
        EUSD.transferShares(msg.sender, eUSDShare);

Please write a coded POC to show how you think this will revert. Otherwise, my judging is finalized here.

Hey @0xean sorry i missed that check u mentioned here, your judging is right and sorry for the confusion!

        uint256 balance = EUSD.sharesOf(address(this));
->      uint256 eUSDShare = balance >= reward ? reward : reward - balance;
        /// @audit if transferShares revert, getReward() will always revert
        EUSD.transferShares(msg.sender, eUSDShare);