`OrigamiLovToken::collectPerformanceFees()` can be frontrunned to extract value

[hats-bug-reporter]

Github username: @osmanozdemir1
Twitter username: osmanozdemir1
Submission hash (on-chain): 0xe92c149ad55bdfa8ee5e8c6a797bdd03b4ff4b7083a4bb00b2f4491e4a3016ed
Severity: medium

Description:
Description
OrigamiLovToken contract has a feature called performance fee where some amount of lovToken is minted as fee periodically based on the total supply at that point. This period is determined as 1 week and it can be assumed these fees will be generated by a lovDSRbot according to protocol's figma architecture.

As we can see below in the collectPerformanceFees() function, performanceFeeAmount of fees will be minted to the feeCollector.

    function collectPerformanceFees() external override onlyElevatedAccess returns (uint256 amount) {
        if (block.timestamp < (lastPerformanceFeeTime + PERFORMANCE_FEE_FREQUENCY)) revert TooSoon();

        address _feeCollector = feeCollector;
        amount = performanceFeeAmount();
        if (amount != 0) {
            emit PerformanceFeesCollected(_feeCollector, amount);
  -->     _mint(_feeCollector, amount); //@audit it will increase the totalSupply of the lovToken.
        }

        lastPerformanceFeeTime = uint32(block.timestamp);
    }

This action will increase the total supply of the lovToken. The amount to mint to the fee collector is calculated with the performanceFeeAmount() function and it is totalSupply * feeBps * 7 days / 365 days / 10_000.

According to the protocol's test files, performance fee bps is initialized as 500 bps. With that value, the amount to mint every week is ~0.1% of the total supply. Which might even be higher if the performance fee bps is increased to greater values like 1000 bps etc.

So here comes the question: Is there a scenario where knowing the 0.1% increase in the lovToken supply be profitable? Short answer: yes.

Origami LovTokenManager contracts are responsible from all of the logic with investing, exiting asset/liability ratios etc. Both _sharesToReserves function and _reservesToShares functions use total supply of the lovToken to calculate how many shares will be minted to the user during investing or how many tokens will be redeemed during exiting.

Let's examine _sharesToReserves:

    function _sharesToReserves(Cache memory cache, uint256 shares) internal view returns (uint256) {
        // If totalSupply is zero, then just return shares 1:1 scaled down to the reserveToken decimals
        // If > 0 then the decimal conversion is handled already (numerator cancels out denominator)
        // Round down for calculating reserves from shares
        return cache.totalSupply == 0
            ? shares.scaleDown(_reservesToSharesScalar(), OrigamiMath.Rounding.ROUND_DOWN)
-->        : shares.mulDiv(_userRedeemableReserves(cache), cache.totalSupply, OrigamiMath.Rounding.ROUND_DOWN); //@audit-issue cache.totalSupply is lovToken supply and it is denominator. Exiting right before totalSupply is increased will result in more tokens to be redeemed.
    }

As we can see above, totalSupply is used during this function and this function is used when exiting. Exiting right before collectPerformanceFee()function is called will:

1. Result in more tokens to be redeemed compared to calling it after.
2. Decrease the total supply of the lovToken because of burning of these tokens, which will result in less fee to be minted to fee collector since the performanceFeeAmount is directly correlated with the supply at that exact moment.

Attack Scenario\

1. Attacker regularly checks performance fee collection actions.
2. Frontruns this call to exit right before total supply is increased.
3. Then backruns this action to invest in again.

I would like to add that there are dynamic fees during investing and exiting. Attacker can quote and only perform this action if there is profit. If the extracted additional value due to increased supply is greater than the exit fee, than the attacker would perform this attack and not do anything otherwise.

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

[osmanozdemir1]

By the way I also would like to add that the attacker does not have to frontrun the exact call. They can check the latest performanceFee collection time, and then exit like 6 days 23 hours 55 minutes after that last collection time. They already know when the supply will increase (a.k.a. when their share value will decrease).

[frontier159]

Thanks for raising, however there are mechanisms in place which make this not possible:

A few points:

• It is not profitable for the would-be exploiter
  • They will be paying a minimum of 0.5% fee to exit the position (which is deflationary to the vault tokens)
  • The performanceFee is going to be about 5.2% (so around ~0.1% per week)
  • So the fee is far greater than the fee they would make
• The performance fee is not intended to be run exactly at the same time each week. The automated bot which will run this will randomize the time which rewards are collected (>= 7 days)
• The collectPerformanceFees() is intended to be called via a private RPC (flashbots protect)