sherlock-audit / 2024-03-axis-finance-judging

1 stars 0 forks source link

FindEverythingX - Unsafe casting for capacityInQuote condition can result in overflow for maxPayout #205

Closed sherlock-admin4 closed 7 months ago

sherlock-admin4 commented 7 months ago

FindEverythingX

medium

Unsafe casting for capacityInQuote condition can result in overflow for maxPayout

Summary

Unsafe casting for capacityInQuote condition can result in overflow for maxPayout

Vulnerability Detail

Contract: FPAM.sol

Within the creation of FPAM auctions, the possibility exists to determine the capacityInQuote variable as boolean value:

https://github.com/sherlock-audit/2024-03-axis-finance/blob/main/moonraker/src/modules/Auction.sol#L60

https://github.com/sherlock-audit/2024-03-axis-finance/blob/main/moonraker/src/bases/Auctioneer.sol#L162

https://github.com/sherlock-audit/2024-03-axis-finance/blob/main/moonraker/src/bases/Auctioneer.sol#L254

From the provided links above, it is clear that this is only possible for one certain execution path: The creation of an FPAM auction without prefunding.

This variable has the purpose to ensure that the capacity as quote tokens cannot be exceeded. Trivially speaking, after a certain threshold of provided quote tokens, users cannot further purchase.

Example:

Alice provides 90e18 quote tokens to purchase base tokens. Afterwards, Bob provides 15e18 quote tokens to purchase base tokens. Bob's purchase will revert because the capacity left is only 10e18.

Now we come to the interesting point, upon auction creation, the _auction function within the FPAM contract is triggered:

https://github.com/sherlock-audit/2024-03-axis-finance/blob/main/moonraker/src/modules/auctions/FPAM.sol#L78

which calculates the maxPayout as follows:

capacity * maxPayoutPercent / _ONE_HUNDRED_PERCENT

if the capacityInQuote boolean is true, the maxPayout will be calculated as follows:

maxPayout * 10^baseTokenDecimals / price

This result of this calculation is then casted to uint96, which is subject to overflow.

https://github.com/sherlock-audit/2024-03-axis-finance/blob/main/moonraker/src/modules/auctions/FPAM.sol#L97

So, let’s reiterate: capacity and therefore also maxPayout can very well be near uint96.max (even though this is rare and depends on the token, it can happen)

This means that in our above mentioned calculation, the possibility of an overflow very well exists.

Consider the example:

maxPayout = 100 000 000e18 baseTokenDecimals = 18 price = 0.0001e18

This yields us the following calculation:

100 000 000e18 * 1e18 / 0.0001e18 = 1e30

which is then casted to uint96.max and therefore overflows.

The result of this blunder is a smaller maxPayout than expected, this can significantly alter the auction behavior and auction fairness.

This issue has only been rated as medium because no funds are lost/stuck in that case but the expected behavior is altered.

Impact

IMPACT:

a) Falsified expected auction behavior

Code Snippet

https://github.com/sherlock-audit/2024-03-axis-finance/blob/main/moonraker/src/modules/Auction.sol#L60 https://github.com/sherlock-audit/2024-03-axis-finance/blob/main/moonraker/src/bases/Auctioneer.sol#L162 https://github.com/sherlock-audit/2024-03-axis-finance/blob/main/moonraker/src/bases/Auctioneer.sol#L254 https://github.com/sherlock-audit/2024-03-axis-finance/blob/main/moonraker/src/modules/auctions/FPAM.sol#L78 https://github.com/sherlock-audit/2024-03-axis-finance/blob/main/moonraker/src/modules/auctions/FPAM.sol#L97

Tool used

Manual Review

Recommendation

Consider simply switching to a uint256 approach, this should be adapted in the overall architecture. The only important thing (as far as I have observed) is to make sure the heap mechanism does not overflow when calculating the relative values:

[https://github.com/sherlock-audit/2024-03-axis-finance/blob/main/moonraker/src/lib/MaxPriorityQueue.sol#L114]

Duplicate of #204

0xJem commented 7 months ago

Low priority.

Possible, but highly unlikely.

nevillehuang commented 7 months ago

Same conditions required as per this [comment](https://github.com/sherlock-audit/2024-03-axis-finance-judging/issues/204#issuecomment-2039134599 to overflow), so duplicate of #204