DenTonylifer - Missing zero amount check may lead to loss of funds

[sherlock-admin]

DenTonylifer

high

Missing zero amount check may lead to loss of funds

Summary

Malicious user can steal accrued interest from other user using redeemWithYT() function.

Vulnerability Detail

This function is used for withdrawing underlying tokens from the caller in exchange for amount of PT and YT. Withdrawn amount will be the sum of the following:

• amount derived from PT + YT burn
• amount of unclaimed yield
• amount of accrued yield from the last time when the YT balance was updated to now If the caller is not from, from must have approved the caller to spend pyAmount.

  function redeemWithYT(address from, address to, uint256 pyAmount) external nonReentrant returns (uint256) {
      //...
      _burnFrom(from, pyAmount);
      _yt.burnFrom(from, msg.sender, pyAmount);

  But any user with zero allowance also can call this function, if he pass pyAmount as 0, in other case function will revert due to _spendAllowance() in internal _burnFrom function:

  function _burnFrom(address owner, uint256 amount) internal {
      if (owner != msg.sender) {
          _spendAllowance(owner, msg.sender, amount);
      }
      _burn(owner, amount);
  }

  In this case withdrawn amount will be the sum of unclaimed yield and accrued yield, and it will be tranfered to to adress, which is malicious user's adress:

  uint256 accruedInTarget = unclaimedYields[from];
  //...
  // Sum up the accrued yield, plus the unclaimed yield from the last time to now
  accruedInTarget += _computeAccruedInterestInTarget(_gscales.maxscale, _lscale, _yt.balanceOf(from));
  //...
  uint256 sharesRedeemed = pyAmount.divWadDown(_gscales.maxscale);  /*---sharesRedeemed will be 0 ---*/
  //...
  // Withdraw underlying tokens from the adapter and transfer them to the user
  _target.safeTransfer(address(adapter), sharesRedeemed + accruedInTarget);
  (uint256 amountWithdrawn, ) = adapter.prefundedRedeem(to);

  Impact

  Malicious user can steal accrued yield from any users.

  Code Snippet

  [https://github.com/sherlock-audit/2024-01-napier/blob/main/napier-v1/src/Tranche.sol#L230-L288]()

  Tool used

Manual Review

Recommendation

Recomended to add zero amount check to prevent calling this function by users with zero allowance, when caller is not from:

+     if (pyAmount == 0) revert ZeroAmount();

Duplicate of #28

[sherlock-admin]

1 comment(s) were left on this issue during the judging contest.

takarez commented:

valid: high(1)

[sherlock-admin4]

The protocol team fixed this issue in PR/commit https://github.com/napierfi/napier-v1/pull/171.

[sherlock-admin4]

The Lead Senior Watson signed off on the fix.