whitehat - liveMarketsBetween reverts if lastIndex_ is out of marketsToAuctioneers length

[sherlock-admin]

whitehat

medium

liveMarketsBetween reverts if lastIndex_ is out of marketsToAuctioneers length

Summary

When the lastIndex parameter of liveMarketsBetween() is bigger than marketsCounter, isLive(id) reverts which cause bad UX.

Vulnerability Detail

To show live markets at Frontend, there is liveMarketsBetween which accepts firstIndex and lastIndex. It can be used for pagination and some other purposes. But as isLive() doesn't check the id is under length and marketsToAuctioneers[id] is valid, it reverts the entire transaction, which causes bad UX and should have been showing available markets instead.

Impact

Bad User Experience

Code Snippet

  function isLive(uint256 id_) public view override returns (bool) {
      IBondAuctioneer auctioneer = marketsToAuctioneers[id_];
      return auctioneer.isLive(id_);
  }

https://github.com/sherlock-audit/2023-02-bond/blob/main/bonds/src/BondAggregator.sol#L132-L159

Tool used

Manual Review

Recommendation

  function isLive(uint256 id_) public view override returns (bool) {
      IBondAuctioneer auctioneer = marketsToAuctioneers[id_];
      if (address(auctioneer) == address(0)) 
          return false;
      return auctioneer.isLive(id_);
  }

[Oighty]

BondAggregator.sol is out of scope for this audit. See https://github.com/sherlock-audit/2023-02-bond#audit-scope. Additionally, front-ends should use the marketCounter to limit their lastIndex value when paginating. If we add another check, it will just fail elsewhere.