sherlock-audit / 2023-02-bond-judging

2 stars 0 forks source link

xiaoming90 - Incorrect max accepted amount returned from the `BondBaseFPA.maxAmountAccepted` function (FPA Only) #22

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

xiaoming90

high

Incorrect max accepted amount returned from the BondBaseFPA.maxAmountAccepted function (FPA Only)

Summary

The BondBaseFPA.maxAmountAccepted return an incorrect max accepted amount.

Vulnerability Detail

The BondBaseFPA.maxAmountAccepted function relies on the BondBaseFPA.marketPrice function to fetch the market price. The market price is expected to be in the price ratio of quote tokens per payout token (Q/P).

File: BondBaseFPA.sol
369:     function maxAmountAccepted(uint256 id_, address referrer_) external view returns (uint256) {
370:         // Calculate maximum amount of quote tokens that correspond to max bond size
371:         // Maximum of the maxPayout and the remaining capacity converted to quote tokens
372:         BondMarket memory market = markets[id_];
373:         uint256 price = marketPrice(id_);
374:         uint256 quoteCapacity = market.capacityInQuote
375:             ? market.capacity
376:             : market.capacity.mulDiv(price, market.scale);
377:         uint256 maxQuote = market.maxPayout.mulDiv(price, market.scale);
378:         uint256 amountAccepted = quoteCapacity < maxQuote ? quoteCapacity : maxQuote;
379: 
380:         // Take into account teller fees and return
381:         // Estimate fee based on amountAccepted. Fee taken will be slightly larger than
382:         // this given it will be taken off the larger amount, but this avoids rounding
383:         // errors with trying to calculate the exact amount.
384:         // Therefore, the maxAmountAccepted is slightly conservative.
385:         uint256 estimatedFee = amountAccepted.mulDiv(
386:             _teller.getFee(referrer_),
387:             ONE_HUNDRED_PERCENT
388:         );
389: 
390:         return amountAccepted + estimatedFee;
391:     }

However, due to a bug in the BondBaseFPA.marketPrice function, this function returns the price ratio of payout tokens per quote token (P/Q) instead of the ratio of quote tokens per payout token (Q/P). Refer to my other write-up titled "Incorrect market price returned from BondBaseFPA.marketPrice function (FPA Only)" on the bug in the BondBaseFPA.marketPrice function for more details. As a result, the max accepted amount computed will be incorrect.

Impact

If any internal function within the protocol relies on the BondBaseFPA.maxAmountAccepted function, it will receive the wrong result, thus causing a computation error.

In addition, since this is a public function, any external or third-party protocol that integrates with this function would also suffer the same problem.

Code Snippet

https://github.com/sherlock-audit/2023-02-bond/blob/main/bonds/src/bases/BondBaseFPA.sol#L369

Tool used

Manual Review

Recommendation

Remediate the bug in the BondBaseFPA.marketPrice function and ensure that it returns the market price in the format of quote tokens per payout token (Q/P).

Duplicate of #20

Oighty commented 1 year ago

Disagree with this finding. See comment on #20