[BVR-01M] Inexistent Sanitization of Range Validity

BVR-01M: Inexistent Sanitization of Range Validity

Type	Severity	Location
Mathematical Operations		BosonVoucher.sol:L168, L169

Description:

The reserveRange function does not explicitly validate that the range defined can be adequately computed (i.e. does not overflow via addition in issueVoucher).

Impact:

The issue does not pose an active threat, however, this validation should be enforced locally to ensure the security guarantees of the BosonVoucher contract are upheld self-sufficiently.

Example:

/**
 * @notice Initializes the voucher.
 * This function is callable only once.
 */
function initializeVoucher(
    uint256 _sellerId,
    address _newOwner,
    VoucherInitValues calldata voucherInitValues
) public initializer {
    string memory sellerId = Strings.toString(_sellerId);
    string memory voucherName = string(abi.encodePacked(VOUCHER_NAME, " ", sellerId));
    string memory voucherSymbol = string(abi.encodePacked(VOUCHER_SYMBOL, "_", sellerId));

    __ERC721_init_unchained(voucherName, voucherSymbol);

    // we don't call init on ownable, but rather just set the ownership to correct owner
    _transferOwnership(_newOwner);

    _setContractURI(voucherInitValues.contractURI);

    _setRoyaltyPercentage(voucherInitValues.royaltyPercentage);

    emit VoucherInitialized(_sellerId, _royaltyPercentage, _contractURI);
}

/**
 * @notice Issues a voucher to a buyer.
 *
 * Minted voucher supply is sent to the buyer.
 * Caller must have PROTOCOL role.
 *
 * Reverts if:
 * - Exchange id falls within a reserved range
 *
 * @param _exchangeId - the id of the exchange (corresponds to the ERC-721 token id)
 * @param _buyer - the buyer address
 */
function issueVoucher(uint256 _exchangeId, address _buyer) external override onlyRole(PROTOCOL) {
    // Get the exchange
    (, Exchange memory exchange) = getBosonExchange(_exchangeId);

    // See if the offer id is associated with a range
    Range storage range = _rangeByOfferId[exchange.offerId];

    // Revert if exchange id falls within a reserved range
    uint256 rangeStart = range.start;
    require(
        (_exchangeId < rangeStart) || (_exchangeId >= rangeStart + range.length),
        EXCHANGE_ID_IN_RESERVED_RANGE
    );

    // Mint the voucher, sending it to the buyer address
    _mint(_buyer, _exchangeId);
}

/**
 * @notice Burns a voucher.
 *
 * Caller must have PROTOCOL role.
 *
 * @param _exchangeId - the id of the exchange (corresponds to the ERC-721 token id)
 */
function burnVoucher(uint256 _exchangeId) external override onlyRole(PROTOCOL) {
    _burn(_exchangeId);
}

/**
 * @notice Reserves a range of vouchers to be associated with an offer
 *
 * Must happen prior to calling preMint
 * Caller must have PROTOCOL role.
 *
 * Reverts if:
 * - Start id is not greater than zero
 * - Offer id is already associated with a range
 *
 * @param _offerId - the id of the offer
 * @param _start - the first id of the token range
 * @param _length - the length of the range
 */
function reserveRange(
    uint256 _offerId,
    uint256 _start,
    uint256 _length
) external onlyRole(PROTOCOL) {
    // Make sure range start is valid
    require(_start > 0, INVALID_RANGE_START);

    // Get storage slot for the range
    Range storage range = _rangeByOfferId[_offerId];

    // Revert if the offer id is already associated with a range
    require(range.length == 0, OFFER_RANGE_ALREADY_RESERVED);

    // Store the reserved range
    range.offerId = _offerId;
    range.start = _start;
    range.length = _length;
    _rangeOfferIds.push(_offerId);

    emit RangeReserved(_offerId, range);
}

Recommendation:

While this is indirectly protected against via the maximum length limitation wherever reserveRange is invoked, it should also be validated locally as it is currently possible to create an invalid range entry which will be unusable due to an overflow failure.

bosonprotocol / boson-protocol-contracts