transferVoucher and alterVoucher should check that voucher exists

[cducrest]

Summary

The function to transfer and alter a voucher should check that a voucher exists before attempting to transfer / alter it.

## Vulnerability Detail

The functions transferVoucher() and alterVoucher() do not revert when called with a voucher that was not minted beforehand:

    function transferVoucher(
        uint256 voucherID,
        string memory ownerID
    ) public whenNotPaused onlyOwner {
        vouchers[voucherID].ownerID = ownerID;
        emit transferVoucherEvent(voucherID, ownerID);
    }

    function alterVoucher(
        uint256 voucherID,
        SharedStructs.Voucher memory voucher
    ) public whenNotPaused onlyOwner {
        vouchers[voucherID] = voucher;
        emit alterVoucherEvent(voucherID, voucher);
    }

Impact

This could result in confusion to the caller of the contract if they want to mint a voucher and transfer it to someone but they mistakenly call transferVoucher() before calling mintVoucher() resulting in the voucher being owned by the owner provided in mintVoucher(). This could also occur due to a reorg of caller transactions if the owner contract does not enforce an ordering of the calls.

This can lead to unforeseen event handling in the backend / frontend when events are received out of expected order.

References

audit-wiiqare-1/contracts/WiiQareVoucherV1.sol

Lines 82 to 101 in 0243110

	function transferVoucher(
	uint256 voucherID,
	string memory ownerID
	) public whenNotPaused onlyOwner {
	vouchers[voucherID].ownerID = ownerID;
	emit transferVoucherEvent(voucherID, ownerID);
	}
	/**
	* Allows the contract owner to alter data for a voucher when the contract is not paused
	* @param voucherID id of the target voucher
	* @param voucher new voucher data
	*/
	function alterVoucher(
	uint256 voucherID,
	SharedStructs.Voucher memory voucher
	) public whenNotPaused onlyOwner {
	vouchers[voucherID] = voucher;
	emit alterVoucherEvent(voucherID, voucher);
	}

Recommendation

Revert when the function is called with a non-existing voucher, e.g. by checking that vouchers[voucherID].value > 0 or with ERC721's _exists(voucherID) function.

[liviuvlad-innovatorspark]

_exists was removed for v5 of the ERC721, so it was replaced by the following modifier
require(ownerOf(voucherID) != address(0), "Voucher doesn't exist!");
thanks for the great feedback, issue is being addressed here