sherlock-audit / 2023-02-fair-funding-judging

1 stars 0 forks source link

oxcm - [H] The protocol does not properly distribute past profits between existing and new shareholders #112

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

oxcm

high

[H] The protocol does not properly distribute past profits between existing and new shareholders

Summary

The contract allows users to invest ETH into the protocol and receive a proportional amount of ETH tokens in return as claimable amount accumulate over time.

However, the contract does not ensure equal profit distribution for new and existing token holders. Specifically, the contract does not settle past profits before issuing new shares, which allows new shareholders to immediately claim profits that were generated prior to their investment.

Vulnerability Detail

This issue arises from the fact that the contract does not consider the historical profits generated by the protocol when issuing new shares.

When a user invests ETH and receives share in return, the smart contract records the number of share issued, but does not settle the past profits generated by the protocol.This means that new shareholders can claim profits that were generated prior to their investment by call withdraw_underlying_to_claim.

POC

  1. Alice invests 100 ETH and borrows 50 alETH.
  2. As investment profits accumulate, the amount of the debt for 50 alETH is automatically repaid, reducing it to 48 alETH. At this point, the protocol generates a maximum claimable amount of 4 ETH.
  3. Bob invests 100 ETH and borrows 50 alETH.
  4. Bob calls withdraw_underlying_to_claim to withdraw the 4 ETH profit from Alchemix. The profit will be split evenly among all current shares. Bob then immediately calls claim, which entitles him to about 2 ETH of the profit. However, these profits actually belong to Alice.

Impact

Existing shareholders will receive less expected ETH return, as a portion of the accumulated past profits will be taken by new shares.

Code Snippet

https://github.com/sherlock-audit/2023-02-fair-funding/blob/main/fair-funding/contracts/Vault.vy#L393-L404

@external
def withdraw_underlying_to_claim(_amount_shares: uint256, _min_weth_out: uint256):
    """
    @notice
        Withdraws _amount_shares and _min_weth_out from Alchemix to be distributed
        to token holders.
        The WETH is held in this contract until it is `claim`ed.
    """
    amount_withdrawn: uint256 = self._withdraw_underlying_from_alchemix(_amount_shares, self, _min_weth_out)
    self._mark_as_claimable(amount_withdrawn)

    log Claimable(amount_withdrawn)

Tool used

Manual Review / ChatGPT PLUS

Recommendation

To address this issue, recommend updated contract implement settling all outstanding profits before issuing new shares

Unstoppable-DeFi commented 1 year ago

Conceptually the auction phase of a Fair Funding campaign will be limited and rather short (for example first Fair Funding campaign will run 16 days / 16 auctions).

Compared to the available APYs on Alchemix (2-5% max), the discrepancy between early depositors and late depositors is effectively near zero and negligible.

hrishibhat commented 1 year ago

Closing based on Sponsor comment, as the impact would be low