InventoryStaking deposit() and withdraw() don't validate passed vaultId

[code423n4]

Handle

Ruhum

Vulnerability details

Impact

In NFTXInventoryStaking a user can withdraw() and deposit() for a vaultId they specify. But, the vaultId is not validated. Instead, the transaction will simply fail at some point because the vault is a zero address. That leads to a bad user experience since the user loses more gas than is really necessary. Instead, the function should validate that there is a vault with that specific ID and revert if that's not the case.

Proof of Concept

https://github.com/code-423n4/2021-12-nftx/blob/main/nftx-protocol-v2/contracts/solidity/NFTXInventoryStaking.sol#L84

https://github.com/code-423n4/2021-12-nftx/blob/main/nftx-protocol-v2/contracts/solidity/NFTXInventoryStaking.sol#L104

Both functions will at some point try to access the passed vault which could be a zero address:
For withdraw() it happens here: https://github.com/code-423n4/2021-12-nftx/blob/main/nftx-protocol-v2/contracts/solidity/NFTXInventoryStaking.sol#L105

For deposit() it happens here: https://github.com/code-423n4/2021-12-nftx/blob/main/nftx-protocol-v2/contracts/solidity/NFTXInventoryStaking.sol#L149

Tools Used

none

Recommended Mitigation Steps

Add the following require statement at the beginning of the function:
require(nftxVaultFactory.vault(vaultId) != address(0), "invalid vaultId");