xiaoming90 - `BondAggregator.findMarketFor` Function Will Break In Certain Conditions

[sherlock-admin]

xiaoming90

medium

BondAggregator.findMarketFor Function Will Break In Certain Conditions

Summary

BondAggregator.findMarketFor function will break when the BondBaseSDA.payoutFor function within the for-loop reverts under certain conditions.

Vulnerability Detail

The BondBaseSDA.payoutFor function will revert if the computed payout is larger than the market's max payout. Refer to Line 711 below.

https://github.com/sherlock-audit/2022-11-bond/blob/main/src/bases/BondBaseSDA.sol#L699

File: BondBaseSDA.sol
699:     function payoutFor(
700:         uint256 amount_,
701:         uint256 id_,
702:         address referrer_
703:     ) public view override returns (uint256) {
704:         // Calculate the payout for the given amount of tokens
705:         uint256 fee = amount_.mulDiv(_teller.getFee(referrer_), 1e5);
706:         uint256 payout = (amount_ - fee).mulDiv(markets[id_].scale, marketPrice(id_));
707: 
708:         // Check that the payout is less than or equal to the maximum payout,
709:         // Revert if not, otherwise return the payout
710:         if (payout > markets[id_].maxPayout) {
711:             revert Auctioneer_MaxPayoutExceeded();
712:         } else {
713:             return payout;
714:         }
715:     }

The BondAggregator.findMarketFor function will call the BondBaseSDA.payoutFor function at Line 245. The BondBaseSDA.payoutFor function will revert if the final computed payout is larger than the markets[id_].maxPayout as mentioned earlier. This will cause the entire for-loop to "break" and the transaction to revert.

Assume that the user configures the minAmountOut_ to be 0, then the condition minAmountOut_ <= maxPayout Line 244 will always be true. The amountIn_ will always be passed to the payoutFor function. In some markets where the computed payout is larger than the market's max payout, the BondAggregator.findMarketFor function will revert.

https://github.com/sherlock-audit/2022-11-bond/blob/main/src/BondAggregator.sol#L221

File: BondAggregator.sol
220:     /// @inheritdoc IBondAggregator
221:     function findMarketFor(
222:         address payout_,
223:         address quote_,
224:         uint256 amountIn_,
225:         uint256 minAmountOut_,
226:         uint256 maxExpiry_
227:     ) external view returns (uint256) {
228:         uint256[] memory ids = marketsFor(payout_, quote_);
229:         uint256 len = ids.length;
230:         uint256[] memory payouts = new uint256[](len);
231: 
232:         uint256 highestOut;
233:         uint256 id = type(uint256).max; // set to max so an empty set doesn't return 0, the first index
234:         uint48 vesting;
235:         uint256 maxPayout;
236:         IBondAuctioneer auctioneer;
237:         for (uint256 i; i < len; ++i) {
238:             auctioneer = marketsToAuctioneers[ids[i]];
239:             (, , , , vesting, maxPayout) = auctioneer.getMarketInfoForPurchase(ids[i]);
240: 
241:             uint256 expiry = (vesting <= MAX_FIXED_TERM) ? block.timestamp + vesting : vesting;
242: 
243:             if (expiry <= maxExpiry_) {
244:                 payouts[i] = minAmountOut_ <= maxPayout
245:                     ? payoutFor(amountIn_, ids[i], address(0))
246:                     : 0;
247: 
248:                 if (payouts[i] > highestOut) {
249:                     highestOut = payouts[i];
250:                     id = ids[i];
251:                 }
252:             }
253:         }
254: 
255:         return id;
256:     }

Impact

The find market feature within the protocol is broken under certain conditions. As such, users would not be able to obtain the list of markets that meet their requirements. The market makers affected by this issue will lose the opportunity to sell their bond tokens.

Code Snippet

https://github.com/sherlock-audit/2022-11-bond/blob/main/src/bases/BondBaseSDA.sol#L699

https://github.com/sherlock-audit/2022-11-bond/blob/main/src/BondAggregator.sol#L221

Tool used

Manual Review

Recommendation

Consider using try-catch or address.call to handle the revert of the BondBaseSDA.payoutFor function within the for-loop gracefully. This ensures that a single revert of the BondBaseSDA.payoutFor function will not affect the entire for-loop within the BondAggregator.findMarketFor function.

[Evert0x]

Message from sponsor

---

Agree with this finding. We implemented the try-catch suggestion without the findMarketFor for loop to silently handle these errors while still preserving the error for other uses.

[xiaoming9090]

Fixed in https://github.com/Bond-Protocol/bonds/commit/56a11260c40785509215b8c81830f6270b46d15f