code-423n4 / 2023-01-popcorn-findings

0 stars 0 forks source link

`AdapterBase.accruedPerformanceFee` does not work with tokens with low decimals #802

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/vault/adapter/abstracts/AdapterBase.sol#L531

Vulnerability details

The performance fees are only accrued when shareValue > highWaterMark_.

529: function accruedPerformanceFee() public view returns (uint256) {
530:         uint256 highWaterMark_ = highWaterMark;
531:         uint256 shareValue = convertToAssets(1e18); //@audit M: this does not work for assets with low decimals. will never accrue any fee
532:         uint256 performanceFee_ = performanceFee;
533: 
534:         return
535:             performanceFee_ > 0 && shareValue > highWaterMark_
536:                 ? performanceFee_.mulDiv(
537:                     (shareValue - highWaterMark_) * totalSupply(),
538:                     1e36,
539:                     Math.Rounding.Down
540:                 )
541:                 : 0;
542:     }

highWaterMark is set in the initializer as 1e18.

The issue is that the value returned by convertToAssets will be in assets.decimals

ERC4626Upgradeable:

168:     function _convertToAssets(uint256 shares, MathUpgradeable.Rounding rounding) internal view virtual returns (uint256 assets) {
169:         uint256 supply = totalSupply();
170:         return
171:             (supply == 0)
172:                 ? shares.mulDiv(10**_asset.decimals(), 10**decimals(), rounding)
173:                 : shares.mulDiv(totalAssets(), supply, rounding);
174:     }

This means the performance fee will never be accrued for a token with low decimals:

Take for example USDC as an asset. Even if it reaches a share value of 1.5:1, the value returned will be 1.5*1e6, which is much lower than 1e18, meaning no performance fee is accrued, while it should.

Impact

Medium

Tools Used

Manual Analysis

Mitigation

highWaterMark should be set to 10**asset.decimals(), not 1e18

c4-judge commented 1 year ago

dmvt marked the issue as duplicate of #252

c4-sponsor commented 1 year ago

RedVeil marked the issue as sponsor confirmed

c4-judge commented 1 year ago

dmvt marked the issue as partial-50

c4-judge commented 1 year ago

dmvt marked the issue as duplicate of #365

c4-judge commented 1 year ago

dmvt marked the issue as not a duplicate

c4-judge commented 1 year ago

dmvt marked the issue as duplicate of #306

c4-judge commented 1 year ago

dmvt marked the issue as satisfactory