Factory update logic of option NFT enables owner to steal funds
Description
Each existing option corresponds to a WasabiOption NFT. For access control purposes, the contract stores the address of the corresponding WasabiPoolFactory. The factory provides an interface for pools to mint new option NFTs. However, it is important to note that the factory address can be upgraded in a way that allows the owner to potentially harm both individual option holders and all holders.
Specifically, to remove existing positions, the owner can first call setFactory on the associated option NFT. This gives them access to the burn function:
function burn(uint256 _optionId) external {
require(msg.sender == factory, "Only the factory can burn tokens");
_burn(_optionId);
}After they burn a given option NFT, the owner can use setFactory to replace the correct factory address and resume pool mechanics.
Impact
When the owner burns option NFTs, it effectively denies their holders the right to exercise the option they purchased. Since ownership of these NFTs is checked during execution, it is crucial to ensure that the holder's rights are respected and they can exercise their options as intended.
function validateOptionForExecution(uint256 _optionId, uint256 _tokenId) private {
require(optionIds.contains(_optionId), "WasabiPool: Option NFT doesn't belong to this pool");
require(_msgSender() == optionNFT.ownerOf(_optionId), "WasabiPool: Only the token owner can execute the option");
WasabiStructs.OptionData memory optionData = options[_optionId];
require(optionData.expiry >= block.timestamp, "WasabiPool: Option has expired");
if (optionData.optionType == WasabiStructs.OptionType.CALL) {
validateAndWithdrawPayment(optionData.strikePrice, "WasabiPool: Strike price needs to be supplied to execute a CALL option");
} else if (optionData.optionType == WasabiStructs.OptionType.PUT) {
require(_msgSender() == nft.ownerOf(_tokenId), "WasabiPool: Need to own the token to sell in order to execute a PUT option");
}
}Further, the owner can prevent all holders from exercising options simply by fixing the factory address at a different value. Executing an option requires it to be successfully burned, and the factory loses the right to do so:
function clearOption(uint256 _optionId, uint256 _tokenId, bool _executed) internal {
WasabiStructs.OptionData memory optionData = options[_optionId];
if (optionData.optionType == WasabiStructs.OptionType.CALL) {
if (_executed) {
// Sell to executor, the validateOptionForExecution already checked if strike is paid
nft.safeTransferFrom(address(this), _msgSender(), optionData.tokenId);
tokenIds.remove(optionData.tokenId);
}
if (tokenIdToOptionId[optionData.tokenId] == _optionId) {
delete tokenIdToOptionId[optionData.tokenId];
}
} else if (optionData.optionType == WasabiStructs.OptionType.PUT) {
if (_executed) {
// Buy from executor
nft.safeTransferFrom(_msgSender(), address(this), _tokenId);
payAddress(_msgSender(), optionData.strikePrice);
}
}
options[_optionId].active = false;
factory.burnOption(_optionId);
}Recommendations
We recommend that Wasabi implement one of the following solutions:
Prevent factory upgrades in the WasabiOption NFT, or
Support multiple factories, allowing them to be added but not removed. This
would also require more granular access control (specifically, storing which
NFTs a given factory is permitted to burn).
Remediation
This issue has been acknowledged by Wasabi, and a fix was implemented in commit 1aca0ac1↗.